This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6b136b19cbe4: [Power10] Implement custom codegen for the
vec_replace_elt and… (authored by amyk).
Changed prior to commit:
nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.
LGTM. Thanks for your patience and for addressing all the comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83500/new/
amyk updated this revision to Diff 293779.
amyk added a comment.
- Updated the `isEltOfVectorTy()` to the correct semantics; making it take in a
vector type and then getting the element type within the function.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
nemanjai added inline comments.
Comment at: clang/lib/Sema/SemaChecking.cpp:3165
+static bool isEltOfVectorTy(ASTContext , CallExpr *Call, Sema ,
+QualType VectorEltTy, QualType EltTy) {
I think this should actually take a vector
amyk added a comment.
@nemanjai Would you please take another look to see if I have addressed your
comments when you get a chance? Thanks.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83500/new/
https://reviews.llvm.org/D83500
amyk updated this revision to Diff 293313.
amyk added a comment.
Address Nemanja's review comments:
- More specific comments when bitcasting the inputs
- Pull out conditions to bitcast the input, use ternary op depending if the
input is 32 or 64-bits
- Create new static function to check if a
nemanjai requested changes to this revision.
nemanjai added a comment.
This revision now requires changes to proceed.
Just marking this not ready to keep my queue clean until the comments are
addressed.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
nemanjai added inline comments.
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14267
+assert((ArgWidth == 32 || ArgWidth == 64) && "Invalid argument width");
+if (BuiltinID == PPC::BI__builtin_altivec_vec_replace_elt)
+ ConstArg *= ArgWidth / 8;
`//
amyk added a comment.
@nemanjai Could you please take another look to see if I have addressed your
comments?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83500/new/
https://reviews.llvm.org/D83500
___
amyk updated this revision to Diff 287444.
amyk added a comment.
Update to address clang-format.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83500/new/
https://reviews.llvm.org/D83500
Files:
clang/include/clang/Basic/BuiltinsPPC.def
amyk updated this revision to Diff 287142.
amyk added a comment.
Address review comments:
- Further consolidate the custom codegen of the two builtins
- Add SemaChecking for if the third argument is a constant, if the third
argument is in range and if the second argument is the same type as the
nemanjai requested changes to this revision.
nemanjai added a comment.
This revision now requires changes to proceed.
The description includes `... however it is more preferable to use bitcast`. It
is not a question of preference but of correctness. The fp to int conversions
truncate while
kamaub accepted this revision as: kamaub.
kamaub 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/D83500/new/
https://reviews.llvm.org/D83500
amyk updated this revision to Diff 278019.
amyk edited the summary of this revision.
amyk added a parent revision: D83497: [PowerPC][Power10] Fix VINS* (vector
insert byte/half/word) instructions to have i32 arguments..
amyk added a comment.
Corrected the patch as it previously caused errors to
amyk updated this revision to Diff 276853.
amyk added a comment.
Fix assignment of variable.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83500/new/
https://reviews.llvm.org/D83500
Files:
clang/include/clang/Basic/BuiltinsPPC.def
amyk updated this revision to Diff 276844.
amyk added a comment.
Address review comments
- update comments
- pull out common code
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83500/new/
https://reviews.llvm.org/D83500
Files:
amyk marked 3 inline comments as done.
amyk added inline comments.
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14273
+// The third argument to vec_replace_elt will be emitted to either
+// the vinsw or vinsd instruction. It must be a compile time constant.
+
lei added inline comments.
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14273
+// The third argument to vec_replace_elt will be emitted to either
+// the vinsw or vinsd instruction. It must be a compile time constant.
+ConstantInt *ArgCI = dyn_cast(Ops[2]);
amyk updated this revision to Diff 276804.
amyk added a comment.
Updated for clang format changes.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83500/new/
https://reviews.llvm.org/D83500
Files:
clang/include/clang/Basic/BuiltinsPPC.def
clang/lib/CodeGen/CGBuiltin.cpp
amyk marked 2 inline comments as done.
amyk added inline comments.
Comment at: clang/include/clang/Basic/BuiltinsPPC.def:339
+BUILTIN(__builtin_altivec_vec_replace_elt, "V4UiV4UiULLiIi", "t")
+BUILTIN(__builtin_altivec_vec_replace_unaligned, "V4UiV4UiULLiIi", "t")
amyk created this revision.
amyk added reviewers: PowerPC, power-llvm-team, nemanjai, lei.
amyk added projects: LLVM, PowerPC, clang.
Herald added a subscriber: shchenz.
This patch implements custom codegen for the `vec_replace_elt` and
`vec_replace_unaligned` builtins.
These builtins map to
21 matches
Mail list logo
