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

craig.topper accepted this revision. craig.topper added a comment. This revision is now accepted and ready to land. LGTM 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

BeMg updated this revision to Diff 558134. BeMg added a comment. 1. reuse the find result 2. replace for loop with llvm::copy_if 3. use explicit std::vector declaration instead of auto Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151730/new/

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

craig.topper added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:229 +collectNonISAExtFeature(const std::vector , int XLen) { + auto I = llvm::find(FeaturesVec, "__RISCV_TargetAttrNeedOverride"); + auto FeatureNeedOveride = std::vector(FeaturesVec.begin(),

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

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

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

BeMg added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:370 + std::vector ) { + Features.push_back("__RISCV_TargetAttrNeedOverride"); + auto RII = llvm::RISCVISAInfo::parseArchString( craig.topper wrote: > Why

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

BeMg updated this revision to Diff 557995. BeMg added a comment. 1. remain else if by removing continue 2. Keep NonISAExtFeature after override 3. Update testcase with -target-feature +save-restore 4. Improve resolveTargetOverride function Repository: rG LLVM Github Monorepo CHANGES SINCE

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

craig.topper added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:370 + std::vector ) { + Features.push_back("__RISCV_TargetAttrNeedOverride"); + auto RII = llvm::RISCVISAInfo::parseArchString( Why do we need

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

BeMg added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:434 +if (MarchFromCPU != "") { + Ret.Features.clear(); + handleFullArchString(MarchFromCPU, Ret.Features); craig.topper wrote: > Why does this clear Ret.Features,

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

craig.topper added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:229 +resolveTargetAttrOverride(const std::vector ) { + if (!llvm::is_contained(FeaturesVec, "__RISCV_TargetAttrNeedOverride")) +return FeaturesVec; Can we use something like

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

BeMg updated this revision to Diff 557943. BeMg added a comment. Herald added a reviewer: aaron.ballman. Align with spec 1. duplicate target attribute will cause compilation fail Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151730/new/

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

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

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

BeMg updated this revision to Diff 556544. BeMg added a comment. Update to lastest spec 1. When it exist the duplicate target attribute, select the lastest one. 2. arch's features will override cpu's features. TargetFeature will override commandline feature. 3. enhence the testcase

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

BeMg updated this revision to Diff 552265. BeMg added a comment. Also remove the `-` operator from target attribute Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151730/new/ https://reviews.llvm.org/D151730 Files:

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

wangpc added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:376 + + if (AttrString.startswith("+") || AttrString.startswith("-")) { +// EXTENSION like arch=+v,+zbb,-c `-` is removed in

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

BeMg updated this revision to Diff 552200. BeMg added a comment. Herald added a subscriber: sunshaoce. Remove some condition because RFC doesn't support it. (Must include prefix arch|cpu|tune) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

craig.topper added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:417 + continue; +} else if (Feature.startswith("no-")) + Ret.Features.push_back("-" + Feature.split("-").second.str()); Is this tested? I don't see any "no-" in the

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

BeMg updated this revision to Diff 550237. BeMg added a comment. rebase and update testcase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151730/new/ https://reviews.llvm.org/D151730 Files: clang/lib/Basic/Targets/RISCV.cpp

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

BeMg updated this revision to Diff 539907. BeMg added a comment. Move riscv-func-attr-target.ll into D155155 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151730/new/ https://reviews.llvm.org/D151730 Files:

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

BeMg updated this revision to Diff 539901. BeMg added a comment. Update testcase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151730/new/ https://reviews.llvm.org/D151730 Files: clang/lib/Basic/Targets/RISCV.cpp

[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

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

BeMg updated this revision to Diff 539856. BeMg added a comment. separate the change to RISCVAsmPrinter.cpp into another patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151730/new/ https://reviews.llvm.org/D151730 Files:

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

reames added a comment. In D151730#4491786 , @craig.topper wrote: > In D151730#4491773 , @jrtc27 wrote: > >> Isn't multiversioning a separate thing that builds on top of per-function >> target attributes? > >

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

BeMg updated this revision to Diff 539428. BeMg added a comment. 1. simplify riscv-func-attr-target.ll 2. handle corner case in parseTargetAttr Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151730/new/ https://reviews.llvm.org/D151730 Files:

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

craig.topper added a comment. In D151730#4491773 , @jrtc27 wrote: > Isn't multiversioning a separate thing that builds on top of per-function > target attributes? That's what I thought. The supportsMultiVersioning call was in an earlier version of the

[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

reames added a comment. FYI, this change appears to be a partial diff. Locally, I need to add RISCV to supportsMultiVersioning() to get this to work at all. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151730/new/

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

reames added a comment. Can you separate the change to RISCVAsmPrinter.cpp into it's own review? This looks useful for any case where we have functions in the same model with different function attributes. The __attribute__((target...) syntax is one way to have that, but there are also

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

BeMg updated this revision to Diff 535653. BeMg added a comment. Use getTargetFeatureForExtension instead of isSupportExtension Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151730/new/ https://reviews.llvm.org/D151730 Files:

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

BeMg added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:385 + StringRef ExtName = Ext.substr(1); + if (llvm::RISCVISAInfo::isSupportedExtensionWithVersion(ExtName) || + llvm::RISCVISAInfo::isSupportedExtension(ExtName))

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

craig.topper added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:385 + StringRef ExtName = Ext.substr(1); + if (llvm::RISCVISAInfo::isSupportedExtensionWithVersion(ExtName) || + llvm::RISCVISAInfo::isSupportedExtension(ExtName))

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

craig.topper added inline comments. Comment at: clang/lib/Basic/Targets/RISCV.cpp:385 + StringRef ExtName = Ext.substr(1); + if (llvm::RISCVISAInfo::isSupportedExtensionWithVersion(ExtName) || + llvm::RISCVISAInfo::isSupportedExtension(ExtName))

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

BeMg updated this revision to Diff 535191. BeMg marked 8 inline comments as done. BeMg added a comment. Add const Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151730/new/ https://reviews.llvm.org/D151730 Files:

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

BeMg updated this revision to Diff 534428. BeMg added a comment. 1. Attrstring -> AttrString 2. using StringRef instead of string 3. Append unsupported target feature Let follow-up procedure raise diagnostic code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

craig.topper added inline comments. Herald added a subscriber: wangpc. Comment at: clang/lib/Basic/Targets/RISCV.cpp:369 +Feature = Feature.trim(); +StringRef Attrstring = Feature.split("=").second.trim(); + Attrstring -> AttrString

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

BeMg updated this revision to Diff 529906. BeMg added a comment. 1. Update testcase 2. Remove supportsMultiVersioning for RISCV Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151730/new/ https://reviews.llvm.org/D151730 Files:

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

craig.topper added inline comments. Comment at: clang/include/clang/Basic/TargetInfo.h:1401 +return getTriple().isX86() || getTriple().isAArch64() || + getTriple().isRISCV(); } Is this needed for target attribute? The description to me reads

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

MaskRay added inline comments. Comment at: clang/test/CodeGen/RISCV/riscv-func-attr-target.c:2 +// REQUIRES: riscv-registered-target +// RUN: %clang -target riscv64 -march=rv64g -S %s -o - | FileCheck %s --check-prefix=CHECK-ASM +// RUN: %clang -target riscv64 -march=rv64g

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

BeMg updated this revision to Diff 529510. BeMg added a comment. Use isSupportedExtensionWithVersion to implement parseTargetAttr Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151730/new/ https://reviews.llvm.org/D151730 Files:

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

BeMg updated this revision to Diff 528390. BeMg added a comment. 1. Update testcase 2. Support - 3. Verfy extension version Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151730/new/ https://reviews.llvm.org/D151730 Files:

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

kito-cheng added inline comments. Comment at: clang/test/CodeGen/RISCV/riscv-func-attr-target.c:10 +// CHECK-ASM: .option pop +__attribute__((target("arch=rv64g,+c,+v"))) void test1 () {} + It's ext list or full arch, can't mixed together. ``` ARCH-ATTR :=

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

BeMg updated this revision to Diff 527748. BeMg added a comment. 1. Update RTS's function name 2. Add backend testcase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151730/new/ https://reviews.llvm.org/D151730 Files:

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

kito-cheng added a comment. Testcase for backend? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151730/new/ https://reviews.llvm.org/D151730 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

BeMg created this revision. Herald added subscribers: jobnoorman, luke, VincentWu, vkmr, frasercrmck, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, kito-cheng, niosHD,