[PATCH] D99152: [AMX] Prototype for vector and amx bitcast.

fhahn added a comment. In D99152#2661296 , @LuoYuanke wrote: >> Unfortunately this is not possible to use an opaque type with the AMX >> intrinsics at the moment, because of the way they are define. It is possible >> to use opaque types with intrinsics i

[PATCH] D99433: [Matrix] Including __builtin_matrix_multiply_add for the matrix type extension.

fhahn added a comment. In D99433#2661919 , @everton.constantino wrote: > @fhahn Ok I see what you mean now, this sounds like a doable path and might > be able to cover architectures with specialized matrix multiplication > instructions as well . > > Jus

[PATCH] D99433: [Matrix] Including __builtin_matrix_multiply_add for the matrix type extension.

fhahn added a comment. In D99433#2661357 , @everton.constantino wrote: > @fhahn When I mentioned the splats I was talking about the IR, not the final > code. On the Godbolts links you sent, its the same that I see. However take a > look into the IR your

[PATCH] D99152: [AMX] Prototype for vector and amx bitcast.

fhahn added a comment. In D99152#2655880 , @LuoYuanke wrote: >> Whether to further optimizations are correct is a different problem, but we >> need a specification for the builtins, intrinsics and the type before going >> any further in that direction. >

[PATCH] D99506: [OpenMP][NFC] Move the `noinline` to the parallel entry point

fhahn added a comment. Is it possible to add a test? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99506/new/ https://reviews.llvm.org/D99506 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https

[PATCH] D99152: [AMX] Prototype for vector and amx bitcast.

fhahn added a comment. In D99152#2655691 , @LuoYuanke wrote: >> I think that point was not really clear during the discussion. Using `load >> <256 x i32>` to lower `__tile_loadd() ` would indeed be incorrect. But I >> don't think that's happening at the

[PATCH] D99152: [AMX] Prototype for vector and amx bitcast.

fhahn added a comment. In D99152#2655371 , @lebedev.ri wrote: > In D99152#2655357 , @fhahn wrote: > >> snip >> I'm not sure if the loads and store are actually incorrect. `_tile1024i ` is >> defined as `typedef

[PATCH] D99152: [AMX] Prototype for vector and amx bitcast.

fhahn added a comment. In D99152#2655304 , @lebedev.ri wrote: > In D99152#2655274 , @fhahn wrote: > >> In D99152#2649520 , @LuoYuanke >> wrote: >> >>> In D99152#2647681

[PATCH] D99152: [AMX] Prototype for vector and amx bitcast.

fhahn added a comment. In D99152#2649520 , @LuoYuanke wrote: > In D99152#2647681 , @fhahn wrote: > >> I can't see any `load <256 x i32>` in the linked example, just a store. >> Could you check the example? > > I cr

[PATCH] D99433: [Matrix] Including __builtin_matrix_multiply_add for the matrix type extension.

fhahn added a comment. FYI I filed https://bugs.llvm.org/show_bug.cgi?id=49738 and https://bugs.llvm.org/show_bug.cgi?id=49739 for improving fast-math handling in the lowering pass and support for `pargma fp`, in case you are interested. @everton.constantino out of curiosity, what architecture

[PATCH] D99433: [Matrix] Including __builtin_matrix_multiply_add for the matrix type extension.

fhahn added a comment. In D99433#2653586 , @everton.constantino wrote: > @fhahn That was my first idea however its not as simple as it looks. I tried > moving the adds but splats make it considerably harder to find a pattern that > catches this and fuse

[PATCH] D99433: [Matrix] Including __builtin_matrix_multiply_add for the matrix type extension.

fhahn added a comment. Thanks for putting up the patch! Do you think it would be possible to get the desired behavior without a new builtin? We should be able to combine the add with the initial multiply for each vector, as long as we have the right fast-math flags? IIUC reassociate should be

[PATCH] D99037: [Matrix] Implement explicit type conversions for matrix types

fhahn added a comment. In D99037#2649065 , @SaurabhJha wrote: > Hey @fhahn , > > I realise, as you pointed out before, that we need to do some changes in > CodeGen too. I think its going to be more involved so before starting on > that, it would be great

[PATCH] D99152: [AMX] Prototype for vector and amx bitcast.

fhahn added a comment. > Front-end alloca <256 x i32> for the local variable tile. When the return > value of __builtin_ia32_tileloadd64_internal is assigned to tile. Front-end > bitcast x86_amx to <256 x i32>. The x86_amx is the type returned from > __builtin_ia32_tileloadd64_internal. Can

[PATCH] D99152: [AMX] Prototype for vector and amx bitcast.

fhahn added a comment. In D99152#2644373 , @LuoYuanke wrote: >> To be honest i don't really understand why `x86_amx` type is even there. >> It seems to me that if you just directly used >> `@llvm.x86.tileloadd64.internal` / `@llvm.x86.tilestored64.interna

[PATCH] D99037: [Matrix] Implement explicit type conversions for matrix types

fhahn added a comment. Thanks for the patch! I think this also needs changes in code-gen & code-gen tests. In D99037#2642598 , @SaurabhJha wrote: > In D99037#2640388 , @fhahn wrote: > >> Thanks for the patch! I th

[PATCH] D99037: [Matrix] Implement explicit type conversions for matrix types

fhahn requested changes to this revision. fhahn added reviewers: rjmccall, rsmith, erichkeane. fhahn added a comment. This revision now requires changes to proceed. Thanks for the patch! I think this also needs changes in code-gen & code-gen tests. Comment at: clang/include/cl

[PATCH] D98757: [AMX] Not fold constant bitcast into amx intrisic

fhahn added a comment. In D98757#2633396 , @xiangzhangllvm wrote: > In D98757#2631042 , @lebedev.ri > wrote: > >> The ongoing special-casing of `X86_AMXTy` through the llvm due to the >> inability of the existing

[PATCH] D96418: [clang] Refactor mustprogress handling, add it to all loops in c++11+.

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

[PATCH] D97857: [Matrix] Add support for matrix-by-scalar division.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc92ec0dd92ba: [Matrix] Add support for matrix-by-scalar division. (authored by fhahn). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D97857: [Matrix] Add support for matrix-by-scalar division.

fhahn updated this revision to Diff 33. fhahn added a comment. Thank you very much John! I applied your suggestions to the wording and removed the unneeded language feature. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97857/new/ https://revi

[PATCH] D96850: [clang] Add -ffinite-loops & -fno-finite-loops options.

fhahn abandoned this revision. fhahn added a comment. those have been picked onto 12.x Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96850/new/ https://reviews.llvm.org/D96850 ___ cfe-commits mailing lis

[PATCH] D97857: [Matrix] Add support for matrix-by-scalar division.

fhahn added inline comments. Comment at: clang/test/CodeGen/matrix-type-operators.c:303 } +// CHECK-LABEL: @divide_double_matrix_scalar_float( everton.constantino wrote: > Shouldn't a test for half floating point be added here as well? Thanks for taking a loo

[PATCH] D97857: [Matrix] Add support for matrix-by-scalar division.

fhahn updated this revision to Diff 329596. fhahn added a comment. rebase & added a matrix_types_scalar_division feature, which allows users to check if the current version of clang is new enough to support it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D96418: [clang] Refactor mustprogress handling, add it to all loops in c++11+.

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

[PATCH] D98055: [ExtVectorType] Support conditional select operator for C++.

This revision was automatically updated to reflect the committed changes. fhahn marked an inline comment as done. Closed by commit rGfc8d3766d721: [ExtVectorType] Support conditional select operator for C++. (authored by fhahn). Changed prior to commit: https://reviews.llvm.org/D98055?vs=328815

[PATCH] D98055: [ExtVectorType] Support conditional select operator for C++.

fhahn marked an inline comment as done. fhahn added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:6172-6174 + if (IsVectorConditional) { +return CheckVectorConditionalTypes(Cond, LHS, RHS, QuestionLoc); + } aaron.ballman wrote: > I'll fix tha

[PATCH] D98075: [Matrix] Implement += and -= for MatrixType

This revision was automatically updated to reflect the committed changes. Closed by commit rG63851a701eac: [Matrix] Implement += and -= for MatrixType. (authored by SaurabhJha, committed by fhahn). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98075/

[PATCH] D98075: [Matrix] Implement += and -= for MatrixType

fhahn accepted this revision. fhahn added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98075/new/ https://reviews.llvm.org/D98075 __

[PATCH] D98055: [ExtVectorType] Support conditional select operator for C++.

fhahn added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:5951 const QualType EltTy = - cast(CondTy.getCanonicalType())->getElementType(); + CondTy->isVectorType() + ? cast(CondTy.getCanonicalType())->getElementType() erichkean

[PATCH] D98055: [ExtVectorType] Support conditional select operator for C++.

fhahn updated this revision to Diff 328815. fhahn added a comment. Got rid of the template parameter, greatly simplified the code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98055/new/ https://reviews.llvm.org/D98055 Files: clang/include/clan

[PATCH] D98075: [Matrix] Implement += and -= for MatrixType

fhahn added reviewers: rjmccall, erichkeane. fhahn added a comment. Thanks for the patch! Could you also extend the Sema `matrix-type-operators` tests with checks for `+=` & `-=`? It looks like the handling for multiply is slightly different and already works but is missing test coverage. It wo

[PATCH] D98055: [ExtVectorType] Support conditional select operator for C++.

fhahn created this revision. fhahn added reviewers: rjmccall, erichkeane, aaron.ballman. fhahn requested review of this revision. Herald added a project: clang. This patch implements the conditional select operator for ext_vector_types in C++. It does so by using the same semantics as for C. D714

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.attachedcall' instead of explicitly emitting retainRV or claimRV calls in the IR

fhahn added a comment. In D92808#2602687 , @hans wrote: > In D92808#2601408 , @fhahn wrote: > >> In D92808#2600245 , @hans wrote: >> >>> Reverted in >>> https://github.com/l

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.attachedcall' instead of explicitly emitting retainRV or claimRV calls in the IR

fhahn added a comment. In D92808#2600245 , @hans wrote: > Reverted in > https://github.com/llvm/llvm-project/commit/0a5dd067181dac2a8882a139ea3bd19bdea5fa44 > until this can be fixed. Thanks! I pushed a fix for the issue in 75805dce5ff8

[PATCH] D97857: [Matrix] Add support for matrix-by-scalar division.

fhahn created this revision. fhahn added reviewers: anemet, rjmccall, rsmith, Bigcheese. Herald added subscribers: dexonsmith, tschuett. fhahn requested review of this revision. Herald added projects: clang, LLVM. Herald added a subscriber: llvm-commits. This patch extends the matrix spec to allow

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.attachedcall' instead of explicitly emitting retainRV or claimRV calls in the IR

fhahn added a comment. In D92808#2600031 , @thakis wrote: >> Thank you. Let me know when you have more information. > > Repro is moving along. > https://bugs.chromium.org/p/chromium/issues/detail?id=1182642#c26 and onward > are getting pretty close. Tha

[PATCH] D96419: [clang] Add -ffinite-loops & -fno-finite-loops options.

fhahn added a comment. In D96419#2556411 , @xbolva00 wrote: > In D96419#2556349 , @fhahn wrote: > >> In D96419#213 , @xbolva00 wrote: >> >>> Maybe we want these options i

[PATCH] D96850: [clang] Add -ffinite-loops & -fno-finite-loops options.

fhahn created this revision. Herald added subscribers: jansvoboda11, dexonsmith, dang. fhahn requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This cherry-picks the following patches on the release branch: 6280bb4cd80e

[PATCH] D96761: [clang][cli] Generate -f[no-]finite-loops arguments

fhahn accepted this revision. fhahn added a comment. This revision is now accepted and ready to land. LGTM thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96761/new/ https://reviews.llvm.org/D96761 ___

[PATCH] D96418: [clang] Refactor mustprogress handling, add it to all loops in c++11+.

fhahn updated this revision to Diff 323568. fhahn added a comment. Rebased on top of 51bf4c0e6d4c , which added -ffinite-loops. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D9641

[PATCH] D96419: [clang] Add -ffinite-loops & -fno-finite-loops options.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG51bf4c0e6d4c: [clang] Add -ffinite-loops & -fno-finite-loops options. (authored by fhahn). Changed prior to commit: https://reviews.llvm.org/D9641

[PATCH] D96419: [clang] Add -ffinite-loops & -fno-finite-loops options.

fhahn updated this revision to Diff 323373. fhahn added a comment. Rebased the patch to apply wihtout D96418 . Will land soon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96419/new/ https://reviews.llvm.org/D964

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly emitting retainRV or claimRV calls in the IR

fhahn accepted this revision. fhahn added a comment. This revision is now accepted and ready to land. LGTM as initial step, but it would be good to adjust the name as Duncan suggested before landing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly emitting retainRV or claimRV calls in the IR

fhahn added a comment. Another thing I noticed that there's verifier support missing. I think we should at least check that only a single `clang.arc.rv` bundle is specified (https://github.com/llvm/llvm-project/blob/main/llvm/lib/IR/Verifier.cpp#L3191). We should probably also enforce that the

[PATCH] D96418: [clang] Refactor mustprogress handling, add it to all loops in c++11+.

fhahn updated this revision to Diff 323003. fhahn added a comment. Add reference to standards. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96418/new/ https://reviews.llvm.org/D96418 Files: clang/lib/CodeGen/CGStmt.cpp clang/lib/CodeGen/CodeG

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly emitting retainRV or claimRV calls in the IR

fhahn added a comment. In D92808#2555868 , @ahatanak wrote: > In D92808#2555737 , @dexonsmith > wrote: > >> In D92808#2552354 , @rjmccall wrote: >> >>> The ultimate code-g

[PATCH] D96418: [clang] Refactor mustprogress handling, add it to all loops in c++11+.

fhahn added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.h:523 return getLangOpts().C11 || getLangOpts().C17 || getLangOpts().C2x; } aaron.ballman wrote: > jdoerfert wrote: > > fhahn wrote: > > > jdoerfert wrote: > > > > Can you modify

[PATCH] D96419: [clang] Add -ffinite-loops & -fno-finite-loops options.

fhahn added a comment. In D96419#213 , @xbolva00 wrote: > Maybe we want these options in llvm 12 as well? > > (+ Release note) That's a good idea. The patch currently depends on D96418 . Not sure if we should pull tha

[PATCH] D96419: [clang] Add -ffinite-loops & -fno-finite-loops options.

fhahn added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.h:147 +No = 2, // No loop is assumed to be finite. + }; + jdoerfert wrote: > Can we have different names? > > `FiniteLoopsKind::None` sounds more like what ``FiniteLoopsKind

[PATCH] D96419: [clang] Add -ffinite-loops & -fno-finite-loops options.

fhahn updated this revision to Diff 322803. fhahn added a comment. adjust naming of enum members as suggested, thanks! I think it cannot be an enum class unfortunately because the CodeGenOption machinery seems to use regular enums/unsigneds. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D96418: [clang] Refactor mustprogress handling, add it to all loops in c++11+.

fhahn added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.h:523 return getLangOpts().C11 || getLangOpts().C17 || getLangOpts().C2x; } jdoerfert wrote: > Can you modify the documentation to talk about what loops must make progress, > th

[PATCH] D96418: [clang] Refactor mustprogress handling, add it to all loops in c++11+.

fhahn updated this revision to Diff 322791. fhahn added a comment. Herald added a subscriber: jfb. Document reasoning. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96418/new/ https://reviews.llvm.org/D96418 Files: clang/lib/CodeGen/CGStmt.cpp

[PATCH] D96419: [clang] Add -ffinite-loops & -fno-finite-loops options.

fhahn added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1923 + if (Arg *A = Args.getLastArg(OPT_save_temps_EQ)) +Opts.SaveTempsFilePrefix = xbolva00 wrote: > Not needed Removed, thanks! Repository: rG LLVM Github Monorepo CH

[PATCH] D96419: [clang] Add -ffinite-loops & -fno-finite-loops options.

fhahn updated this revision to Diff 322683. fhahn added a comment. Remove accidentally copied code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96419/new/ https://reviews.llvm.org/D96419 Files: clang/include/clang/Basic/CodeGenOptions.def cl

[PATCH] D96419: [clang] Add -ffinite-loops & -fno-finite-loops options.

fhahn updated this revision to Diff 322679. fhahn added a comment. Add description for options. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96419/new/ https://reviews.llvm.org/D96419 Files: clang/include/clang/Basic/CodeGenOptions.def clang/

[PATCH] D96419: [clang] Add -ffinite-loops & -fno-finite-loops options.

fhahn created this revision. fhahn added reviewers: jdoerfert, rjmccall, xbolva00, atmnpatel, aaron.ballman, rsmith. Herald added subscribers: dexonsmith, dang. Herald added a reviewer: jansvoboda11. fhahn requested review of this revision. Herald added a project: clang. Herald added a subscriber:

[PATCH] D96418: [clang] Refactor mustprogress handling, add it to all loops in c++11+.

fhahn created this revision. fhahn added reviewers: jdoerfert, rjmccall, xbolva00, atmnpatel, aaron.ballman, rsmith. fhahn requested review of this revision. Herald added a project: clang. Currently Clang does not add mustprogress to inifinite loops with a known constant condition, matching C11 b

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

fhahn added a comment. In D94367#2542809 , @atmnpatel wrote: > Wait actually, we're gonna need D94366 > anyways, I'll get address @xbolva00's comments by eod. FWIW I think we should sort this one (D94367

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

fhahn added a comment. Herald added a reviewer: jansvoboda11. @atmnpatel reverse ping. Just curious if you think you'll have time to follow up on this patch soonish? Otherwise I'm probably going to add at least the `-fno-finite-loops` option, because we have users which code breaks due to the `

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly emitting retainRV or claimRV calls in the IR

fhahn accepted this revision. fhahn added a comment. This revision is now accepted and ready to land. Core LLVM parts LGTM, thanks! I'm going to mark this as accepted, given that the other parts have been reviewed earlier already Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly emitting retainRV or claimRV calls in the IR

fhahn added a comment. Thanks for the updates! The LLVM side of things now looks good to me. Could you also add a description of the new bundle to LangRef https://llvm.org/docs/LangRef.html#operand-bundles? In D92808#2535593 , @ahatanak wrote: > We coul

[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly emitting retainRV or claimRV calls in the IR

fhahn added a comment. Thanks for the update! The approach with operand bundles and the no-op uses looks cleaner than the original version. The special handling in the inliner seems a bit unfortunate, but I guess there's no way around that? (as a side note, it might be easier to submit if this

[PATCH] D92808: [ObjC][ARC] Annotate calls with attributes instead of emitting retainRV or claimRV calls in the IR

fhahn added inline comments. Comment at: llvm/lib/IR/Instruction.cpp:580 +if (auto *CB = dyn_cast(this)) + return objcarc::hasRetainRVOrClaimRVAttr(CB); +return false; ahatanak wrote: > fhahn wrote: > > rjmccall wrote: > > > nikic wrote: > > > > This

[PATCH] D92808: [ObjC][ARC] Annotate calls with attributes instead of emitting retainRV or claimRV calls in the IR

fhahn added inline comments. Comment at: llvm/include/llvm/Analysis/ObjCARCRVAttr.h:28 + +static inline const char *getRVAttrValStr(bool Retain) { + return Retain ? "retain" : "claim"; Is there a place the attributes could be documented? Comme

[PATCH] D89699: [ExtVector] Make .even/.odd/.lo/.hi return vectors for single elements.

fhahn abandoned this revision. fhahn added a comment. This unfortunately breaks a bunch of existing projects. I'll abandon the change for now, as the gains are probably not worth the cost of breaking backwards compatibility. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D94779: [Clang] Ensure vector predication pragma is ignored only when vectorization width is 1.

fhahn added a comment. > This ensures that the Clang loop pragma vectorize_predicate([enable|disable]) > is ignored > when vectorize_width(1) is also used, since that effectively disables > vectorization. Could you elaborate *why* it `vectorize_predicate` should be ignored when `vectorize_widt

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

fhahn added reviewers: rjmccall, aaron.ballman, erichkeane. fhahn added a comment. Thanks for putting up the patch! I think the code here and D94366 could be simplified if we would introduce 2 codegen options to distinguish the differences between C and C++:

[PATCH] D86841: [clang] Add mustprogress and llvm.loop.mustprogress attribute deduction

fhahn added a comment. In D86841#2484705 , @atmnpatel wrote: > I'm happy to add a patch amending this, the reason it wasn't done that way > was because at the time and even now, out of icc/clang/msvc/gcc, gcc seems to > be the only one that happily remov

[PATCH] D94083: [AArch64] Add +pauth archictecture option, allowing the v8.3a pointer authentication extension.

fhahn added a comment. FWIW I think it would be good to have a bit more details in the description for changes such as this, like a link to the public docs for the extension. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94083/new/ https://reviews

[PATCH] D86844: [LoopDeletion] Allows deletion of possibly infinite side-effect free loops

fhahn added a comment. In D86844#2484639 , @atmnpatel wrote: > I might be misunderstanding the standard here but since 1 is a non-zero > constant expression, it can't be assumed to terminate by the implementation > right? The relevant section from C11 at

[PATCH] D86841: [clang] Add mustprogress and llvm.loop.mustprogress attribute deduction

fhahn added subscribers: xbolva00, fhahn. fhahn added inline comments. Comment at: clang/lib/CodeGen/CGStmt.cpp:799 EmitBoolCondBranch = false; + FnIsMustProgress = false; +} Shouldn't this only apply for C? http://eel.is/c++draft/intro.progress#1

[PATCH] D86844: [LoopDeletion] Allows deletion of possibly infinite side-effect free loops

fhahn added a comment. In D86844#2481922 , @xbolva00 wrote: > int a, b; > > int f(void) { > while (1) { > if (a != b) return 1; > } > return 0; > } > > int g(int a, int b) { > while (1) { > if (a !=

[PATCH] D93483: Add element-type to the Vector TypeLoc types.

fhahn accepted this revision. fhahn added a comment. This revision is now accepted and ready to land. LGTM, thanks Comment at: clang/include/clang/AST/TypeLoc.h:1756 + +class VectorTypeLoc : public ConcreteTypeLoc { erichkeane wrote: > fhahn wrote: > > Can we r

[PATCH] D94092: [Clang] Remove unnecessary Attr.isArgIdent checks.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7ef9139a391a: [Clang] Remove unnecessary Attr.isArgIdent checks. (authored by fhahn). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D94092: [Clang] Remove unnecessary Attr.isArgIdent checks.

fhahn added a comment. In D94092#2482035 , @erichkeane wrote: > In D94092#2481857 , @aaron.ballman > wrote: > >> In D94092#2481276 , @rjmccall wrote: >> >>> Without botherin

[PATCH] D86844: [LoopDeletion] Allows deletion of possibly infinite side-effect free loops

fhahn added a comment. Now that the test is fixed, this could be re-landed, right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86844/new/ https://reviews.llvm.org/D86844 ___ cfe-commits mailing list cf

[PATCH] D94092: [Clang] Remove unnecessary Attr.isArgIdent checks.

fhahn updated this revision to Diff 314640. fhahn added a comment. Address Simon's comments, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94092/new/ https://reviews.llvm.org/D94092 Files: clang/lib/Sema/SemaType.cpp Index: clang/lib/Se

[PATCH] D94092: [Clang] Remove unnecessary Attr.isArgIdent checks.

fhahn added a comment. In D94092#2479753 , @erichkeane wrote: >> I tried a few different things to construct matrix_type attributes with >> ArgIdents, but failed. The patch also adjusts the code for a bunch of >> attributes. So if there are indeed cases

[PATCH] D94092: [Clang] Remove unnecessary Attr.isArgIdent checks.

fhahn added a comment. Comment at: clang/lib/Sema/SemaType.cpp:7661 - Expr *SizeExpr; - // Special case where the argument is a template id. - if (Attr.isArgIdent(0)) { -CXXScopeSpec SS; -SourceLocation TemplateKWLoc; -UnqualifiedId Id; -Id.setIdentifier(A

[PATCH] D94092: [Clang] Remove unnecessary Attr.isArgIdent checks.

fhahn updated this revision to Diff 314635. fhahn added a comment. In D94092#2479684 , @erichkeane wrote: > I'm not sure how well Attr.td's constraints are enforced on type attributes, > as these often happen before parsing is completely done. I'd imag

[PATCH] D72281: [Matrix] Add matrix type to Clang.

fhahn added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:7855 + return; +RowsExpr = Columns.get(); + } else { fhahn wrote: > RKSimon wrote: > > @fhahn Should this be ColsExpr? > > ``` > > ColsExpr = Columns.get(); > > ``` > > Noticed when lo

[PATCH] D94092: [Clang] Remove unnecessary Attr.isArgIdent checks.

fhahn updated this revision to Diff 314632. fhahn added a comment. Also remove Attr.isArgIdent check from AddressSpaceTypeAttribute Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94092/new/ https://reviews.llvm.org/D94092 Files: clang/lib/Sema/Se

[PATCH] D94092: [Clang] Remove unnecessary Attr.isArgIdent checks.

fhahn created this revision. fhahn added reviewers: aaron.ballman, rjmccall, Bigcheese, erichkeane. fhahn requested review of this revision. Herald added a project: clang. The MatrixType, ExtVectorType and VectorSize attributes have arguments defined as ExprArguments in Attr.td. So their arguments

[PATCH] D93014: [Clang] Add AArch64 VCMLA LANE variants.

This revision was automatically updated to reflect the committed changes. Closed by commit rG51d5991f04dd: [Clang] Add AArch64 VCMLA LANE variants. (authored by fhahn). Changed prior to commit: https://reviews.llvm.org/D93014?vs=314452&id=314620#toc Repository: rG LLVM Github Monorepo CHANG

[PATCH] D93483: Add element-type to the Vector TypeLoc types.

fhahn added inline comments. Comment at: clang/include/clang/AST/TypeLoc.h:1756 + +class VectorTypeLoc : public ConcreteTypeLoc { Can we reuse/unify this with `MatrixTypeLoc`? And then have `MatrixTypeLoc` just deal with the row/column operands. CHANGES SINCE

[PATCH] D93014: [Clang] Add AArch64 VCMLA LANE variants.

fhahn added inline comments. Comment at: clang/include/clang/Basic/arm_neon.td:1911 +// vcmlaq{ROT}_lane +def : SOpInst<"vcmla" # ROT # "_lane", "...qI", "Q" # type, Op<(call "vcmla" # ROT, $p0, $p1, + (bitcast $p0, (dup_typed laneqty , (call "vget_lane", (bitc

[PATCH] D93014: [Clang] Add AArch64 VCMLA LANE variants.

fhahn updated this revision to Diff 314452. fhahn added a comment. Add tests for rotated lane variants. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93014/new/ https://reviews.llvm.org/D93014 Files: clang/include/clang/Basic/arm_neon.td clang

[PATCH] D93014: [Clang] Add AArch64 VCMLA LANE variants.

fhahn updated this revision to Diff 314389. fhahn added a comment. Add comment to tblgen change, remove trailing whitespace. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93014/new/ https://reviews.llvm.org/D93014 Files: clang/include/clang/Basi

[PATCH] D72281: [Matrix] Add matrix type to Clang.

fhahn added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:7855 + return; +RowsExpr = Columns.get(); + } else { RKSimon wrote: > @fhahn Should this be ColsExpr? > ``` > ColsExpr = Columns.get(); > ``` > Noticed when looking at > https://llvm.

[PATCH] D93952: [Clang][Misc] Fix fragile test

fhahn added a comment. In D93952#2475526 , @atmnpatel wrote: > I can't say that I know with any certainty, I'm too inexperienced. This > failure was missed by me locally, the pre-merge bot, and by most of the > buildbots other than the ones I mentioned a

[PATCH] D86844: [LoopDeletion] Allows deletion of possibly infinite side-effect free loops

fhahn accepted this revision. fhahn added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: clang/test/Misc/loop-opt-setup.c:26 // Check br i1 to make sure the loop is gone, there will still be a label branch for the infinite loop. // CH

[PATCH] D86844: [LoopDeletion] Allows deletion of possibly infinite side-effect free loops

fhahn added inline comments. Comment at: llvm/test/Other/loop-deletion-printer.ll:17 -define void @deleteme() { +define void @deleteme() willreturn { entry: atmnpatel wrote: > fhahn wrote: > > Is this change related to the patch? Same for the other test change

[PATCH] D86844: [LoopDeletion] Allows deletion of possibly infinite side-effect free loops

fhahn added inline comments. Comment at: clang/test/Misc/loop-opt-setup.c:26 // Check br i1 to make sure the loop is gone, there will still be a label branch for the infinite loop. // CHECK-LABEL: Helper This comment needs updating, there's no loop there now

[PATCH] D91444: [InstCombine] Preserve !annotation on newly created instructions.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG01089c876bff: [InstCombine] Preserve !annotation on newly created instructions. (authored by fhahn). Changed prior to commit: https://reviews.llvm

[PATCH] D91444: [InstCombine] Preserve !annotation metadata for memory combines.

fhahn updated this revision to Diff 312216. fhahn added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. In D91444#2394512 , @lebedev.ri wrote: > What about teaching IRBuilder to deal with it like it deals with debugloc? D

[PATCH] D92930: [Clang] Add vcmla and rotated variants for Arm ACLE.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9c4cddb53a7b: [Clang] Add vcmla and rotated variants for Arm ACLE. (authored by fhahn). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D93014: [Clang] Add AArch64 VCMLA LANE variants.

fhahn created this revision. fhahn added reviewers: SjoerdMeijer, dmgreen, t.p.northover. Herald added subscribers: danielkiss, kristof.beyls. fhahn requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This patch adds the LANE variants for VCMLA

[PATCH] D92930: [Clang] Add vcmla and rotated variants for Arm ACLE.

fhahn updated this revision to Diff 310799. fhahn added a comment. Loop over rotation variants. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92930/new/ https://reviews.llvm.org/D92930 Files: clang/include/clang/Basic/arm_neon.td clang/lib/Cod

[PATCH] D92930: [Clang] Add vcmla and rotated variants for Arm ACLE.

fhahn added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:5237 NEONMAP1(vcaddq_rot90_v, arm_neon_vcadd_rot90, Add1ArgType), + NEONMAP1(vcaddq_rot90_v, arm_neon_vcadd_rot90, Add1ArgType), NEONMAP1(vcage_v, arm_neon_vacge, 0), t.p.northover wr

<    1   2   3   4   5   6   7   8   >