[PATCH] D155688: [PATCH] [llvm] [InstCombine] Reassociate loop invariant GEP index calculations.

paulwalker-arm added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:2332 } - + if (GEP.getNumIndices() == 1 && !GEP.getType()->isVectorTy()) { +auto *Idx = dyn_cast(GEP.getOperand(1)); Perhaps move this block after t

[PATCH] D158479: [clang] Support elementwise builtin for sizeless vector type

paulwalker-arm added inline comments. Comment at: clang/test/CodeGen/aarch64-sve-vector-elementwise-ops.c:128-131 +// CHECK-NEXT:[[ELT_MIN:%.*]] = call @llvm.umin.nxv16i8( [[VI8:%.*]], [[VI8]]) +// CHECK-NEXT:[[ELT_MIN1:%.*]] = call @llvm.umin.nxv8i16( [[VI16:%.*]],

[PATCH] D158045: [clang][SVE] Rename isVLSTBuiltinType, NFC

paulwalker-arm accepted this revision. paulwalker-arm added a comment. This revision is now accepted and ready to land. I'd hope there are common code paths where `isVLSTBuiltinType` would still be useful but I guess this step is required to make that happen anyway. If you permit a minor request

[PATCH] D157269: [Clang][AArch64] Diagnostics for SME attributes when target doesn't have 'sme'

paulwalker-arm added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:6762 +Context.getFunctionFeatureMap(CallerFeatureMap, CallerFD); +if (!CallerFeatureMap.count("sme")) + Diag(Loc, diag::err_sme_call_in_non_sme_target); `co

[PATCH] D153560: [Clang] Allow C++11 style initialisation of SVE types.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG989879f8fded: [Clang] Allow C++11 style initialisation of SVE types. (authored by paulwalker-arm). Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D153560: [Clang] Allow C++11 style initialisation of SVE types.

paulwalker-arm updated this revision to Diff 534914. paulwalker-arm added a comment. Rebased and updated to allow copy initialisation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153560/new/ https://reviews.llvm.org/D153560 Files: clang/lib/Co

[PATCH] D153560: [Clang] Allow C++11 style initialisation of SVE types.

paulwalker-arm added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1878 + +llvm_unreachable("Unexpected initialization of a scalable vector!"); + } efriedma wrote: > paulwalker-arm wrote: > > efriedma wrote: > > > I can see why you can't ha

[PATCH] D153560: [Clang] Allow C++11 style initialisation of SVE types.

paulwalker-arm added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1878 + +llvm_unreachable("Unexpected initialization of a scalable vector!"); + } efriedma wrote: > I can see why you can't have more than one element... but both zero and on

[PATCH] D153560: [Clang] Allow C++11 style initialisation of SVE types.

paulwalker-arm updated this revision to Diff 533631. paulwalker-arm added a comment. Renamed test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153560/new/ https://reviews.llvm.org/D153560 Files: clang/lib/CodeGen/CGExprScalar.cpp clang/test/

[PATCH] D153560: [Clang] Allow C++11 style initialisation of SVE types.

paulwalker-arm updated this revision to Diff 533629. paulwalker-arm added a comment. Moved C++ test into CodeGenCXX. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153560/new/ https://reviews.llvm.org/D153560 Files: clang/lib/CodeGen/CGExprScalar

[PATCH] D153560: [Clang] Allow C++11 style initialisation of SVE types.

paulwalker-arm created this revision. Herald added a project: All. paulwalker-arm requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixes https://github.com/llvm/llvm-project/issues/63223 Repository: rG LLVM Github Monorepo https://revie

[PATCH] D150553: [SVE ACLE] Change the lowering of SVE integer mla_u/mls_u builtins

paulwalker-arm accepted this revision. paulwalker-arm added a comment. This revision is now accepted and ready to land. The commit message is incorrect because the SVE Clang builtins are not named `mla_u` and so should read `SVE integer mla_x/mls_x builtins` Other than that the patch looks good

[PATCH] D148919: [Clang][Sema] Fix invalid cast when validating SVE types within CheckVariableDeclarationType.

This revision was automatically updated to reflect the committed changes. Closed by commit rG2bb7e00b098c: [Clang][Sema] Fix invalid cast when validating SVE types within… (authored by paulwalker-arm). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D14

[PATCH] D148919: [Clang][Sema] Fix invalid cast when validating SVE types within CheckVariableDeclarationType.

paulwalker-arm added reviewers: dmgreen, sdesmalen. paulwalker-arm added a comment. This is not my area so I don't know if replacing `isFunctionOrMethod()` rather than just extending the if clause is a bad idea, but all the tests still pass including the new one that would trigger an assert prio

[PATCH] D148919: [Clang][Sema] Fix invalid cast when validating SVE types within CheckVariableDeclarationType.

paulwalker-arm created this revision. Herald added a subscriber: tschuett. Herald added a project: All. paulwalker-arm requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: cfe-commits, jplehr, sstefan1. Herald added a project: clang. Fixes #62087 Repos

[PATCH] D146146: [Clang] Stop demoting ElementCount/TypeSize conversion errors to warnings.

This revision was automatically updated to reflect the committed changes. Closed by commit rG62f09d788f9f: [Clang] Stop demoting ElementCount/TypeSize conversion errors to warnings. (authored by paulwalker-arm). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D146146: [Clang] Stop demoting ElementCount/TypeSize conversion errors to warnings.

paulwalker-arm added reviewers: sdesmalen, david-arm, craig.topper, reames. paulwalker-arm added a comment. This option was our pragmatic way to ensure scalable vector based toolchains remained useful whilst the kinks were worked out. We've a few releases under our belts now and I feel that eno

[PATCH] D146146: [Clang] Stop demoting ElementCount/TypeSize conversion errors to warnings.

paulwalker-arm created this revision. Herald added a project: All. paulwalker-arm requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D146146 Files: clang/lib/Driver/Too

[PATCH] D136864: [Clang] Create opaque type for AArch64 SVE2p1/SME2 svcount_t.

paulwalker-arm added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:730 ASTContext::BuiltinVectorTypeInfo Info = - CGM.getContext().getBuiltinVectorTypeInfo(BT); - unsigned NumElemsPerVG = (Info.EC.getKnownMinValue() * Info.NumVectors) / 2; +

[PATCH] D143767: [SVE][Builtins] Lower X forms of fp binop/mla arithmetic builtins to dedicated intrinsics

paulwalker-arm added a comment. In D143767#4138049 , @dewen wrote: > Hi, thank you. When will this patch be pushed to the master?@paulwalker-arm I expect to push the patch to main in the next day or so. I just need to look over some recent InstCombine

[PATCH] D141240: [SVE][Builtins] Add metadata to intrinsic calls for builtins that don't define the result of inactive lanes.

paulwalker-arm abandoned this revision. paulwalker-arm added a comment. D141939 turned out to be the better approach. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141240/new/ https://reviews.llvm.org/D141240 ___

[PATCH] D141939: [SVE][Builtins] Lower X forms of binop arithmetic builtins to dedicated intrinsics.

paulwalker-arm added inline comments. Comment at: clang/include/clang/Basic/arm_sve.td:762 -multiclass SInstZPZZ flags=[]> { - def _M : SInst; - def _X : SInst; - def _Z : SInst; - - def _N_M : SInst; - def _N_X : SInst; - def _N_Z : SInst; -} - -defm SVABD_S : SIn

[PATCH] D141939: [SVE][Builtins] Lower X forms of binop arithmetic builtins to dedicated intrinsics.

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

[PATCH] D142065: [SVE] Fix incorrect lowering of predicate permute builtins.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG26b79ca3fafc: [SVE] Fix incorrect lowering of predicate permute builtins. (authored by paulwalker-arm). Changed prior to commit: https://reviews.l

[PATCH] D142065: [SVE] Fix incorrect lowering of predicate permute builtins.

paulwalker-arm added inline comments. Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:6361 + def : SVE_2_Op_Pat(NAME # _S)>; + def : SVE_2_Op_Pat(NAME # _D)>; } peterwaller-arm wrote: > Out of interest, is there a good reason to handle the nxv16 patte

[PATCH] D142065: [SVE] Fix incorrect lowering of predicate permute builtins.

paulwalker-arm added reviewers: david-arm, CarolineConcatto, peterwaller-arm. paulwalker-arm added a comment. This is bug fix based on something spotted when reviewing D141469 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D142065: [SVE] Fix incorrect lowering of predicate permute builtins.

paulwalker-arm created this revision. Herald added subscribers: psnobl, hiraditya, tschuett. Herald added a reviewer: efriedma. Herald added a project: All. paulwalker-arm requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Wh

[PATCH] D141056: [SVE][CGBuiltins] Remove need for instcombine from ACLE tests.

This revision was automatically updated to reflect the committed changes. paulwalker-arm marked an inline comment as done. Closed by commit rG909ac0e97dcb: [SVE][CGBuiltins] Remove need for instcombine from ACLE tests. (authored by paulwalker-arm). Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D141240: [SVE][Builtins] Add metadata to intrinsic calls for builtins that don't define the result of inactive lanes.

paulwalker-arm added a comment. Just a heads up that I'm likely to abandon this patch because as predicted implementing dedicated intrinsics is looking like the better design and most all the code generation plumbing is already present and so even the implementation is minimal. Repository:

[PATCH] D141240: [SVE][Builtins] Add metadata to intrinsic calls for builtins that don't define the result of inactive lanes.

paulwalker-arm added a comment. In D141240#4035438 , @sdesmalen wrote: > Using metadata seems sensible, but did you also identify any downsides? I > could imagine that we'd need to manually propagate metadata to any nodes > after we do a combine (which

[PATCH] D141056: [SVE][CGBuiltins] Remove need for instcombine from ACLE tests.

paulwalker-arm marked an inline comment as done. paulwalker-arm added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:9092 + if (BytesPerElt > 1) { +Value *Scale = ConstantInt::get(Int64Ty, Log2_32(BytesPerElt)); +Ops[2] = Builder.CreateShl(Ops[2

[PATCH] D141056: [SVE][CGBuiltins] Remove need for instcombine from ACLE tests.

paulwalker-arm updated this revision to Diff 488418. paulwalker-arm added a comment. Rebase and use simpler IRBuilder interface. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141056/new/ https://reviews.llvm.org/D141056 Files: clang/lib/CodeGen/

[PATCH] D140983: [IRBuilder] Use canonical i64 type for insertelement index used by vector splats.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGeae26b6640af: [IRBuilder] Use canonical i64 type for insertelement index used by vector… (authored by paulwalker-arm). Changed prior to commit: ht

[PATCH] D141240: [SVE][Builtins] Add metadata to intrinsic calls for builtins that don't define the result of inactive lanes.

paulwalker-arm updated this revision to Diff 487247. paulwalker-arm added a comment. Fixed typo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141240/new/ https://reviews.llvm.org/D141240 Files: clang/lib/CodeGen/CGBuiltin.cpp clang/test/CodeG

[PATCH] D141240: [SVE][Builtins] Add metadata to intrinsic calls for builtins that don't define the result of inactive lanes.

paulwalker-arm created this revision. Herald added subscribers: ctetreau, psnobl, hiraditya, tschuett. Herald added a reviewer: efriedma. Herald added a project: All. paulwalker-arm requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-co

[PATCH] D141056: [SVE][CGBuiltins] Remove need for instcombine from ACLE tests.

paulwalker-arm created this revision. Herald added subscribers: ctetreau, psnobl, arphaman, kristof.beyls, tschuett. Herald added a reviewer: efriedma. Herald added a project: All. paulwalker-arm requested review of this revision. Herald added subscribers: cfe-commits, alextsao1999. Herald added a

[PATCH] D140983: [IRBuilder] Use canonical i64 type for insertelement index used by vector splats.

paulwalker-arm updated this revision to Diff 486332. paulwalker-arm added a comment. Herald added a reviewer: bollu. Update poly tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140983/new/ https://reviews.llvm.org/D140983 Files: clang/test/

[PATCH] D140983: [IRBuilder] Use canonical i64 type for insertelement index used by vector splats.

paulwalker-arm updated this revision to Diff 486284. paulwalker-arm added a comment. Herald added subscribers: Moerafaat, zero9178, bzcheeseman, awarzynski, sdasgup3, wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, stephenneuendorffer, liu

[PATCH] D140983: [IRBuilder] Use canonical i64 type for insertelement index used by vector splats.

paulwalker-arm created this revision. Herald added subscribers: frasercrmck, luismarques, apazos, sameer.abuasal, s.egerton, dmgreen, Jim, jocewei, PkmX, arphaman, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, niosHD, sabuasal, simoncook, johnrusso, rbar, asb, kbarton

[PATCH] D138788: [SVE] Change some bfloat lane intrinsics to use i32 immediates

paulwalker-arm accepted this revision. paulwalker-arm added inline comments. This revision is now accepted and ready to land. Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:1517 -class SVE_4Vec_BF16_Indexed +class SVE_4Vec_BF16_Indexed_V2 : DefaultAttrsIntrinsic<[ll

[PATCH] D138788: [SVE] Change some bfloat lane intrinsics to use i32 immediates

paulwalker-arm added inline comments. Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:2527 -def int_aarch64_sve_bfdot_lane : SVE_4Vec_BF16_Indexed; -def int_aarch64_sve_bfmlalb_lane : SVE_4Vec_BF16_Indexed; -def int_aarch64_sve_bfmlalt_lane : SVE_4Vec_BF16_Indexed; +def

[PATCH] D127762: [Clang][AArch64] Add ACLE attributes for SME.

paulwalker-arm added a comment. Hi @rsandifo-arm , what are your thoughts on Arron's observations? My interpretation is that Arm originally figured the distinction between keywords and gnu attributes was minimal and thus using our previous norms made most sense. This is not my world so my under

[PATCH] D134352: [AArch64] Add Neoverse V2 CPU support

paulwalker-arm accepted this revision. paulwalker-arm added inline comments. Comment at: clang/docs/ReleaseNotes.rst:376 them, which it cannot. +- Add driver and tuning support for Neoverse V2 support via the flag + ``-mcpu=neoverse-v2``. Native detection is also supported vi

[PATCH] D131580: [clang][SVE] Undefine preprocessor macro defined in

This revision was automatically updated to reflect the committed changes. Closed by commit rG48e1250a91d2: [clang][SVE] Undefine preprocessor macro defined in (authored by mgabka, committed by paulwalker-arm). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D131580: [clang][SVE] Undefine preprocessor macro defined in

paulwalker-arm added inline comments. Comment at: clang/utils/TableGen/SveEmitter.cpp:1285 OS << "#endif\n\n"; + OS << "#undef __ai\n\n"; OS << "#endif /*__ARM_FEATURE_SVE */\n\n"; Can you also do this for `__aio`? Repository: rG LLVM Github Monorepo

[PATCH] D127976: [IR] Move vector.insert/vector.extract out of experimental namespace

paulwalker-arm accepted this revision. paulwalker-arm added inline comments. Comment at: llvm/docs/LangRef.rst:17282 + declare @llvm.vector.insert.nxv4f32.v4f32( %vec, <4 x float> %subvec, i64 %idx) + declare @llvm.vector.insert.nxv2f64.v2f64( %vec, <2 x double> %subv

[PATCH] D127655: [AArch64] Define __FP_FAST_FMA[F]

paulwalker-arm added inline comments. Comment at: clang/test/Preprocessor/aarch64-target-features.c:59-60 // CHECK-NOT: __ARM_FEATURE_SVE_BITS 2048 +// CHECK: __FP_FAST_FMA 1 +// CHECK: __FP_FAST_FMAF 1 I don't think we need this change given `init-aarch64.c`

[PATCH] D124998: [AArch64][SVE] Add aarch64_sve_pcs attribute to Clang

paulwalker-arm added a comment. @aaron.ballman It looks like the conversation reached a conclusion? Given this is separate to what we're trying to add here can this patch be unblocked? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124998/new/ htt

[PATCH] D124998: [AArch64][SVE] Add aarch64_sve_pcs attribute to Clang

paulwalker-arm added a comment. In D124998#3494127 , @aaron.ballman wrote: > In D124998#3493791 , > @paulwalker-arm wrote: > >> Just wanted to say this is not a new calling convention as such, but rather >> an

[PATCH] D124998: [AArch64][SVE] Add aarch64_sve_pcs attribute to Clang

paulwalker-arm added a comment. Just wanted to say this is not a new calling convention as such, but rather an existing one that is generally auto-detected based on function signature. The problem we're trying to solve here is that we need a way to allow a user to force the calling convention

[PATCH] D123605: [WIP][Sema][SVE] Move/simplify Sema testing for SVE ACLE builtins

paulwalker-arm added inline comments. Comment at: clang/test/Sema/aarch64-sve2-intrinsics/acle_sve2_imm_n.cpp:25 +{ + // expected-error-re@+1 3 {{argument value {{[0-9]+}} is outside the valid range [0, 7]}} + EXPAND_XZM_FUNC(SVE_ACLE_FUNC(svqshlu,_n_s8,,), pg, svundef_s8(), -

[PATCH] D123605: [WIP][Sema][SVE] Move/simplify Sema testing for SVE ACLE builtins

paulwalker-arm added inline comments. Comment at: clang/test/Sema/aarch64-sve2-intrinsics/acle_sve2_imm_n.cpp:25 +{ + // expected-error-re@+1 3 {{argument value {{[0-9]+}} is outside the valid range [0, 7]}} + EXPAND_XZM_FUNC(SVE_ACLE_FUNC(svqshlu,_n_s8,,), pg, svundef_s8(), -

[PATCH] D123300: [Clang] Enable opaque pointers by default

paulwalker-arm added a comment. We've spotted some breakages caused by this patch within the llvm test suite when built for AArch64-SVE. I've got https://reviews.llvm.org/D123670 as a WIP fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123300/

[PATCH] D123605: Work in progress: [Sema][SVE] Move sema testing for SVE2-AES ACLE builtins

paulwalker-arm added inline comments. Comment at: clang/test/Sema/aarch64-acle-sve2-aes.c:1 +// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve2 -fallow-half-arguments-and-returns -fsyntax-only -std=c99 -verify -verify-ignore-unexpected=error %s + --

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

paulwalker-arm added a comment. Ok, message understood. I'll try and get these cleaned up, I cannot say precisely when but will push for before we branch for 15 if that sounds sensible. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122983/new/ https://reviews.llvm.org/D122983 __

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

paulwalker-arm added a comment. Thanks for this. I can see about cleaning up the tests but I'm still not sure what the advantage is. The affected RUN lines are already `-fsyntax-only` tests. Is it about where the test files live or is there something else I should be considering? The benefi

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

paulwalker-arm added a comment. The tests verify a set of builtins do not exist when the associated feature flag is not present. They sit within CodeGen because the tests were plentiful and it did not seem worth duplicating them. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122983/ne

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

paulwalker-arm added a comment. Please consider this "AArch64 folks speaking up". What are your plans here exactly? I have no issue with adding `-std=c99` to the RUN lines, but "remove the // expected-warning comments" sounds like a significant loss of test coverage. CHANGES SINCE LAST ACTIO

[PATCH] D121829: [clang][AArc64][SVE] Implement vector-scalar operators

paulwalker-arm added a comment. It looks like the issue you fixed for the integer tests also need fixing for the floating point variants. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121829/new/ https://reviews.llvm.org/D121829 _

[PATCH] D119926: [Clang][AArch64] Enable _Float16 _Complex type

paulwalker-arm added inline comments. Comment at: clang/include/clang/AST/ASTContext.h:1117 CanQualType BFloat16Ty; - CanQualType Float16Ty; // C11 extension ISO/IEC TS 18661-3 + CanQualType Float16Ty, Float16ComplexTy; // C11 extension ISO/IEC TS 18661-3 CanQualType Void

[PATCH] D119319: [AArch64] Emit TBAA metadata for SVE load/store intrinsics

paulwalker-arm accepted this revision. paulwalker-arm added a comment. This revision is now accepted and ready to land. I cannot say I fully understand all the connotations of this change but my gut feeling is that if failures occur it's likely the input program is malformed. Either way, we're

[PATCH] D119130: [clangd] NFC: Move stdlib headers handling to Clang

paulwalker-arm added a comment. Yep, that did the trick. Many Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119130/new/ https://reviews.llvm.org/D119130 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D119130: [clangd] NFC: Move stdlib headers handling to Clang

paulwalker-arm added a comment. After this commit I am seeing the link time error ld.lld: error: undefined symbol: clang::DeclContext::isInlineNamespace() const >>> referenced by StandardLibrary.cpp:118 (/home/pmw/projects/upstream-llvm/llvm-project/clang/lib/Tooling/Inclusions/StandardLibra

[PATCH] D107290: [RISCV] Add support for the vscale_range attribute

paulwalker-arm added a comment. Does this mean `RISCVTTIImpl::getMaxVScale()` can be removed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107290/new/ https://reviews.llvm.org/D107290 ___ cfe-commits ma

[PATCH] D115924: [ConstantFolding] Unify handling of load from uniform value

paulwalker-arm added a comment. This patch looks to be breaking the clang-aarch64-sve-vla-2stage buildbot (and probably also clang-aarch64-sve-vls-2stage). I've checked this using `./bin/clang -DNDEBUG -O3 -w -Werror=date-time -w pr19687.c && ./a.out && echo "success"`, which works before this

[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

paulwalker-arm accepted this revision. paulwalker-arm added a comment. This revision is now accepted and ready to land. One minor issue but otherwise looks good. Comment at: clang/lib/Basic/Targets/AArch64.cpp:482 +return std::pair( +LangOpts.VScaleMin ? LangOpts.VS

[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

paulwalker-arm added inline comments. Comment at: clang/lib/Basic/Targets/AArch64.cpp:476-484 + assert(LangOpts.VScaleMin && "vscale min must be greater than 0!"); + + if (LangOpts.VScaleMax) return std::pair(LangOpts.VScaleMin, La

[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

paulwalker-arm added a comment. I agree, it's the change to VScaleMin that has caused the issue. If the LangOpts default can remain as 0 and you can still achieve what you're after then that would be perfect. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113294/new/ https://reviews.l

[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

paulwalker-arm requested changes to this revision. paulwalker-arm added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Basic/Targets/AArch64.cpp:476-484 + assert(LangOpts.VScaleMin && "vscale min must be greater than 0!"); + + if (LangOpts

[PATCH] D114713: [AArch64][SVE][NEON] Add NEON-SVE-Bridge intrinsics

paulwalker-arm added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:1325 setOperationAction(ISD::MLOAD, VT, Custom); + setOperationAction(ISD::INSERT_SUBVECTOR, VT, Custom); } MattDevereau wrote: > paulwalker-arm wro

[PATCH] D114713: [AArch64][SVE][NEON] Add NEON-SVE-Bridge intrinsics

paulwalker-arm added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:1325 setOperationAction(ISD::MLOAD, VT, Custom); + setOperationAction(ISD::INSERT_SUBVECTOR, VT, Custom); } Can you extract this into its own patch

[PATCH] D113489: [AArch64][SVE] Instcombine SVE LD1/ST1 to stock LLVM IR

paulwalker-arm accepted this revision. paulwalker-arm added a comment. This revision is now accepted and ready to land. There's an issue with the a `Value*` being named `VecPtrTy` but otherwise this looks good to me. I'll leave it up to you to decide whether it's worth breaking out the usage of

[PATCH] D113779: [Clang] Add mfp16, mfp16fml and mdotprod flags for ARM target features.

paulwalker-arm added a comment. Rather than adding connivence options after the fact what about allowing `-march=` to be specified multiple times? The first must be the usual format with later ones required to start with `+`. The defined parsing behaviour would be as if there was a single `-ma

[PATCH] D112406: [Driver][AArch64]Add driver support for neoverse-512tvb target

paulwalker-arm added a comment. In D112406#3087191 , @dmgreen wrote: > Thanks. If the cpu has a 512 bit total vector bandwidth, should the > VScaleForTuning be 1 or 2 (or higher)? llvm doesn't usually deal with total > bandwidth a lot, perhaps not as mu

[PATCH] D111790: [AArch64][Driver][SVE] Allow -msve-vector-bits=+ syntax to mean no maximum vscale

paulwalker-arm added inline comments. Comment at: clang/lib/Basic/Targets/AArch64.cpp:454 - if (Opts.ArmSveVectorBits) { -Builder.defineMacro("__ARM_FEATURE_SVE_BITS", Twine(Opts.ArmSveVectorBits)); + if (Opts.VScaleMin) { +Builder.defineMacro("__ARM_FEATURE_SVE_BITS

[PATCH] D111790: [AArch64][Driver][SVE] Allow -msve-vector-bits=+ syntax to mean no maximum vscale

paulwalker-arm added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:7893 // Attribute is unsupported if '-msve-vector-bits=' isn't specified. - if (!S.getLangOpts().ArmSveVectorBits) { + if (!S.getLangOpts().VScaleMin) { S.Diag(Attr.getLoc(), diag::err_attribu

[PATCH] D111790: [AArch64][Driver][SVE] Allow -msve-vector-bits=+ syntax to mean no maximum vscale

paulwalker-arm added a comment. Are the references to "128-bit chunks" for the vscale flags necessary? That's really a nuisance of SVE that LLVM IR should not need to worry about. Can we speak exclusively in terms of vscale or is the "multiples of 128" required somewhere? Perhaps we're missi

[PATCH] D110258: [AArch64][Clang] Always add -tune-cpu argument to -cc1 driver

paulwalker-arm added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1850 + else +TuneCPU = "generic"; +} else What benefit does `-tune-cpu generic` provide? I'm wondering if the patch can be restricted to only add `-tune-cpu`

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

paulwalker-arm added a comment. Please remember to create a more representative commit message as the patch no longer removes getMaxVScale. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106277/new/ https://reviews.llvm.org/D106277 ___

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

paulwalker-arm added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h:134 + unsigned getMaxNumElements(ElementCount VF, + const Instruction *I = nullptr) const { if (!VF.isScalable()) Can this par

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

paulwalker-arm added a comment. @craig.topper can you share RISCV plans around supporting vscale_range? In essence we'd like to deprecate the TTI method and have LLVM IR contain all relevant information when is comes to interpreting vscale. Currently the usage is minimal and so checking both i

[PATCH] D106333: [AArch64][SVE] Handle svbool_t VLST <-> VLAT/GNUT conversion

paulwalker-arm added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:8677 + return Ty->getKind() == BuiltinType::SveBool + ? Context.getLangOpts().ArmSveVectorBits / Context.getCharWidth() + : Context.getLangOpts().ArmSveVectorBits;

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

paulwalker-arm added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:119-131 -Optional RISCVTTIImpl::getMaxVScale() const { - // There is no assumption of the maximum vector length in V specification. - // We use the value specified by users as t

[PATCH] D106333: [AArch64][SVE] Handle svbool_t VLST <-> VLAT/GNUT conversion

paulwalker-arm added a comment. In D106333#2889168 , @junparser wrote: > @efriedma with this patch, all of conversion between VLST and VLAT should > have same vector size(getElementType() * getElementCount()). The regression > in D105097

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

paulwalker-arm added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:505-506 + } else if (getContext().getTargetInfo().hasFeature("sve")) { +CurFn->addFnAttr( +llvm::Attribute::getWithVScaleRangeArgs(getLLVMContext(), 0, 16)); }

[PATCH] D103702: [AArch64][SVE] Wire up vscale_range attribute to SVE min/max vector queries

paulwalker-arm accepted this revision. paulwalker-arm added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.cpp:350-352 unsigned AArch64Subtarget::getMaxSVEVectorSizeInBits() const { assert(HasSVE && "Tried to get SVE vector length without SVE support!")

[PATCH] D103702: [AArch64][SVE] Wire up vscale_range attribute to SVE min/max vector queries

paulwalker-arm added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.h:298-299 + bool LittleEndian, + unsigned MinSVEVectorSizeInBitsOverride = 0, + unsigned MaxSVEVectorSizeInBitsOverride = 0); ---

[PATCH] D101986: [InstSimplify] Remove redundant {insert,extract}_vector intrinsic chains

paulwalker-arm added inline comments. Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:5741-5747 +// (insert_vector _, (extract_vector X, 0), 0) -> X +unsigned IdxN = cast(Idx)->getZExtValue(); +Value *X = nullptr; +if (match(SubVec, m_Intrinsic( +

[PATCH] D98856: Always emit error for wrong interfaces to scalable vectors, unless cmdline flag is passed.

paulwalker-arm accepted this revision. paulwalker-arm added inline comments. Comment at: llvm/include/llvm/Support/TypeSize.h:30 +/// Reports a diagnostic message to indicate a invalid size request has been +/// done on a scalable vector. This function may not return. -

[PATCH] D98856: Always emit error for wrong interfaces to scalable vectors, unless cmdline flag is passed.

paulwalker-arm added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5052-5055 + if (isa(JA)) { +CmdArgs.push_back("-mllvm"); +CmdArgs.push_back("-treat-scalable-fixed-error-as-warning"); + } Are there any concerns related to LTO here

[PATCH] D98487: [AArch64][SVE/NEON] Add support for FROUNDEVEN for both NEON and fixed length SVE

paulwalker-arm accepted this revision. paulwalker-arm added a comment. Mainly focused on the SVE side of things, which looks good to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98487/new/ https://reviews.llvm.org/D98487 _

[PATCH] D98030: [IR] Add vscale_range IR function attribute

paulwalker-arm added inline comments. Comment at: clang/test/CodeGen/arm-sve-vector-bits-vscale-range.c:10-14 +// CHECK-128: attributes #0 = { {{.*}} vscale_range(1,1) {{.*}} } +// CHECK-256: attributes #0 = { {{.*}} vscale_range(2,2) {{.*}} } +// CHECK-512: attributes #0 = { {{.

[PATCH] D98487: [AArch64][SVE/NEON] Add support for FROUNDEVEN for both NEON and fixed length SVE

paulwalker-arm added a comment. In D98487#2625673 , @bsmith wrote: >> Why is this patch only changing int_aarch64_neon_frintn and not >> int_aarch64_sve_frintn? >> Is there a particular reason to do so? > > Things are done slightly differently for SVE in

[PATCH] D98030: [IR] Add vscale_range IR function attribute

paulwalker-arm added inline comments. Comment at: llvm/docs/LangRef.rst:1998 +function. A value of 0 means unbounded. If the optional max value is omitted +then max is set to the value of min. sdesmalen wrote: > Do you need to say anything about a defa

[PATCH] D96852: [clang][SVE] Remove inline keyword from arm_sve.h

paulwalker-arm added a comment. One observation is that for arm_neon.h `__inline__` is used. So perhaps we can just do likewise and we'll also be consistent across the two ACLE headers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96852/new/ htt

[PATCH] D96852: [clang][SVE] Remove inline keyword from arm_sve.h

paulwalker-arm added a comment. In D96852#2568383 , @joechrisellis wrote: > Speaking to @DavidTruby about this, it appears that this fix is insufficient > -- `inline` has important semantic meaning in C++ that means that we can't > simply omit the keywo

[PATCH] D89031: [SVE] Add support to vectorize_width loop pragma for scalable vectors

paulwalker-arm added a comment. As I see it there are a bunch of pragmas that all enable vectorisation, with each pragma providing a unit of information. One component of this information is the vectorisation factor hint provided by vectorize_width. With the introduction of scalable vectors th

[PATCH] D71767: [POC][SVE] Allow code generation for fixed length vectorised loops [Patch 2/2].

paulwalker-arm abandoned this revision. paulwalker-arm added a comment. Herald added a subscriber: ecnelises. With the exception of VSELECT lowering, which is being worked under D85364 , everything else is available in master. Repository: rG LLVM Github Monore

[PATCH] D71760: [POC][SVE] Allow code generation for fixed length vectorised loops [Patch 1/2].

paulwalker-arm abandoned this revision. paulwalker-arm added a comment. The intention of this patch is now complete. All work is available in master with the exception of the hook into -msve-vector-bits which is not necessarily the direction we'll use once function attributes are available. R

[PATCH] D86065: [SVE] Make ElementCount members private

paulwalker-arm added a comment. To be more clear, I'm happy to defer the divide conversation for if/when we run into issues so my previous acceptance still stands. It'll be good to get the intent of the patch in (i.e. stoping access to internal class members) asap, plus any follow up work will

  1   2   >