[PATCH] D70133: [ARM, MVE] Add intrinsics for 'administrative' vector operations.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG902e84556a51: [ARM,MVE] Add intrinsics for
administrative vector operations. (authored by simon_tatham).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70133/new/
https://reviews.llvm.org/D70133
Files:
clang/include/clang/Basic/arm_mve.td
clang/include/clang/Basic/arm_mve_defs.td
clang/test/CodeGen/arm-mve-intrinsics/admin.c
clang/utils/TableGen/MveEmitter.cpp
Index: clang/utils/TableGen/MveEmitter.cpp
===
--- clang/utils/TableGen/MveEmitter.cpp
+++ clang/utils/TableGen/MveEmitter.cpp
@@ -632,10 +632,10 @@
StringRef CallPrefix;
std::vector Args;
std::set AddressArgs;
- std::set IntConstantArgs;
+ std::map IntConstantArgs;
IRBuilderResult(StringRef CallPrefix, std::vector Args,
std::set AddressArgs,
- std::set IntConstantArgs)
+ std::map IntConstantArgs)
: CallPrefix(CallPrefix), Args(Args), AddressArgs(AddressArgs),
IntConstantArgs(IntConstantArgs) {}
void genCode(raw_ostream ,
@@ -644,11 +644,13 @@
const char *Sep = "";
for (unsigned i = 0, e = Args.size(); i < e; ++i) {
Ptr Arg = Args[i];
- if (IntConstantArgs.find(i) != IntConstantArgs.end()) {
+ auto it = IntConstantArgs.find(i);
+ if (it != IntConstantArgs.end()) {
assert(Arg->hasIntegerConstantValue());
-OS << Sep
+OS << Sep << "static_cast<" << it->second << ">("
<< ParamAlloc.allocParam("unsigned",
-utostr(Arg->integerConstantValue()));
+utostr(Arg->integerConstantValue()))
+ << ")";
} else {
OS << Sep << Arg->varname();
}
@@ -763,6 +765,14 @@
// shares with at least one other intrinsic.
std::string ShortName, FullName;
+ // A very small number of intrinsics _only_ have a polymorphic
+ // variant (vuninitializedq taking an unevaluated argument).
+ bool PolymorphicOnly;
+
+ // Another rarely-used flag indicating that the builtin doesn't
+ // evaluate its argument(s) at all.
+ bool NonEvaluating;
+
const Type *ReturnType;
std::vector ArgTypes;
std::map ImmediateArgs;
@@ -796,6 +806,8 @@
return false;
}
bool polymorphic() const { return ShortName != FullName; }
+ bool polymorphicOnly() const { return PolymorphicOnly; }
+ bool nonEvaluating() const { return NonEvaluating; }
// External entry point for code generation, called from MveEmitter.
void genCode(raw_ostream , CodeGenParamAllocator ,
@@ -1126,11 +1138,15 @@
Args.push_back(getCodeForDagArg(D, i, Scope, Param));
if (Op->isSubClassOf("IRBuilderBase")) {
std::set AddressArgs;
- for (unsigned i : Op->getValueAsListOfInts("address_params"))
-AddressArgs.insert(i);
- std::set IntConstantArgs;
- for (unsigned i : Op->getValueAsListOfInts("int_constant_params"))
-IntConstantArgs.insert(i);
+ std::map IntConstantArgs;
+ for (Record *sp : Op->getValueAsListOfDefs("special_params")) {
+unsigned Index = sp->getValueAsInt("index");
+if (sp->isSubClassOf("IRBuilderAddrParam")) {
+ AddressArgs.insert(Index);
+} else if (sp->isSubClassOf("IRBuilderIntParam")) {
+ IntConstantArgs[Index] = sp->getValueAsString("type");
+}
+ }
return std::make_shared(
Op->getValueAsString("prefix"), Args, AddressArgs, IntConstantArgs);
} else if (Op->isSubClassOf("IRIntBase")) {
@@ -1235,6 +1251,9 @@
}
ShortName = join(std::begin(NameParts), std::end(NameParts), "_");
+ PolymorphicOnly = R->getValueAsBit("polymorphicOnly");
+ NonEvaluating = R->getValueAsBit("nonEvaluating");
+
// Process the intrinsic's argument list.
DagInit *ArgsDag = R->getValueAsDag("args");
Result::Scope Scope;
@@ -1404,6 +1423,8 @@
for (bool Polymorphic : {false, true}) {
if (Polymorphic && !Int.polymorphic())
continue;
+ if (!Polymorphic && Int.polymorphicOnly())
+continue;
// We also generate each intrinsic under a name like __arm_vfooq
// (which is in C language implementation namespace, so it's
@@ -1557,7 +1578,10 @@
if (Int.polymorphic()) {
StringRef Name = Int.shortName();
if (ShortNamesSeen.find(Name) == ShortNamesSeen.end()) {
-OS << "BUILTIN(__builtin_arm_mve_" << Name << ", \"vi.\", \"nt\")\n";
+OS << "BUILTIN(__builtin_arm_mve_" << Name << ", \"vi.\", \"nt";
+if (Int.nonEvaluating())
+ OS << "u"; // indicate that this builtin doesn't evaluate its args
+OS << "\")\n";
ShortNamesSeen.insert(Name);
}
}
Index: clang/test/CodeGen/arm-mve-intrinsics/admin.c
===
--- /dev/null
+++
[PATCH] D70133: [ARM, MVE] Add intrinsics for 'administrative' vector operations.
dmgreen accepted this revision.
dmgreen added a comment.
This revision is now accepted and ready to land.
Thanks. That, er, makes it slightly simpler to review.
LGTM
Comment at: clang/test/CodeGen/arm-mve-intrinsics/admin.c:1671
+//
+float16x8_t test_vuninitializedq_polymorphic_f16(float16x8_t (*funcptr)(void))
+{
simon_tatham wrote:
> dmgreen wrote:
> > I see. This is the "non-evaluating" part. It looks odd from a C perspective.
> Slightly, although it's no more odd than `sizeof` or `__typeof` for only
> using the type of its argument and not actually evaluating it. But it's what
> ACLE specifies!
>
> (I think, rather like I just mentioned in D70088, that the use case is for
> polymorphism within something like a C++ template – if you have the vector
> type as a template parameter, this allows you to generate an uninit value of
> the same vector type reasonably easily, without having to write your own
> system of C++ template specializations that select the right explicitly
> suffixed intrinsic.)
It's more like a macro than a function call. Makes sense.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70133/new/
https://reviews.llvm.org/D70133
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70133: [ARM, MVE] Add intrinsics for 'administrative' vector operations.
simon_tatham updated this revision to Diff 229111.
simon_tatham added a comment.
Moved the get/set lane functions out into a separate patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70133/new/
https://reviews.llvm.org/D70133
Files:
clang/include/clang/Basic/arm_mve.td
clang/include/clang/Basic/arm_mve_defs.td
clang/test/CodeGen/arm-mve-intrinsics/admin.c
clang/utils/TableGen/MveEmitter.cpp
Index: clang/utils/TableGen/MveEmitter.cpp
===
--- clang/utils/TableGen/MveEmitter.cpp
+++ clang/utils/TableGen/MveEmitter.cpp
@@ -632,10 +632,10 @@
StringRef CallPrefix;
std::vector Args;
std::set AddressArgs;
- std::set IntConstantArgs;
+ std::map IntConstantArgs;
IRBuilderResult(StringRef CallPrefix, std::vector Args,
std::set AddressArgs,
- std::set IntConstantArgs)
+ std::map IntConstantArgs)
: CallPrefix(CallPrefix), Args(Args), AddressArgs(AddressArgs),
IntConstantArgs(IntConstantArgs) {}
void genCode(raw_ostream ,
@@ -644,11 +644,13 @@
const char *Sep = "";
for (unsigned i = 0, e = Args.size(); i < e; ++i) {
Ptr Arg = Args[i];
- if (IntConstantArgs.find(i) != IntConstantArgs.end()) {
+ auto it = IntConstantArgs.find(i);
+ if (it != IntConstantArgs.end()) {
assert(Arg->hasIntegerConstantValue());
-OS << Sep
+OS << Sep << "static_cast<" << it->second << ">("
<< ParamAlloc.allocParam("unsigned",
-utostr(Arg->integerConstantValue()));
+utostr(Arg->integerConstantValue()))
+ << ")";
} else {
OS << Sep << Arg->varname();
}
@@ -763,6 +765,14 @@
// shares with at least one other intrinsic.
std::string ShortName, FullName;
+ // A very small number of intrinsics _only_ have a polymorphic
+ // variant (vuninitializedq taking an unevaluated argument).
+ bool PolymorphicOnly;
+
+ // Another rarely-used flag indicating that the builtin doesn't
+ // evaluate its argument(s) at all.
+ bool NonEvaluating;
+
const Type *ReturnType;
std::vector ArgTypes;
std::map ImmediateArgs;
@@ -796,6 +806,8 @@
return false;
}
bool polymorphic() const { return ShortName != FullName; }
+ bool polymorphicOnly() const { return PolymorphicOnly; }
+ bool nonEvaluating() const { return NonEvaluating; }
// External entry point for code generation, called from MveEmitter.
void genCode(raw_ostream , CodeGenParamAllocator ,
@@ -1126,11 +1138,15 @@
Args.push_back(getCodeForDagArg(D, i, Scope, Param));
if (Op->isSubClassOf("IRBuilderBase")) {
std::set AddressArgs;
- for (unsigned i : Op->getValueAsListOfInts("address_params"))
-AddressArgs.insert(i);
- std::set IntConstantArgs;
- for (unsigned i : Op->getValueAsListOfInts("int_constant_params"))
-IntConstantArgs.insert(i);
+ std::map IntConstantArgs;
+ for (Record *sp : Op->getValueAsListOfDefs("special_params")) {
+unsigned Index = sp->getValueAsInt("index");
+if (sp->isSubClassOf("IRBuilderAddrParam")) {
+ AddressArgs.insert(Index);
+} else if (sp->isSubClassOf("IRBuilderIntParam")) {
+ IntConstantArgs[Index] = sp->getValueAsString("type");
+}
+ }
return std::make_shared(
Op->getValueAsString("prefix"), Args, AddressArgs, IntConstantArgs);
} else if (Op->isSubClassOf("IRIntBase")) {
@@ -1235,6 +1251,9 @@
}
ShortName = join(std::begin(NameParts), std::end(NameParts), "_");
+ PolymorphicOnly = R->getValueAsBit("polymorphicOnly");
+ NonEvaluating = R->getValueAsBit("nonEvaluating");
+
// Process the intrinsic's argument list.
DagInit *ArgsDag = R->getValueAsDag("args");
Result::Scope Scope;
@@ -1404,6 +1423,8 @@
for (bool Polymorphic : {false, true}) {
if (Polymorphic && !Int.polymorphic())
continue;
+ if (!Polymorphic && Int.polymorphicOnly())
+continue;
// We also generate each intrinsic under a name like __arm_vfooq
// (which is in C language implementation namespace, so it's
@@ -1557,7 +1578,10 @@
if (Int.polymorphic()) {
StringRef Name = Int.shortName();
if (ShortNamesSeen.find(Name) == ShortNamesSeen.end()) {
-OS << "BUILTIN(__builtin_arm_mve_" << Name << ", \"vi.\", \"nt\")\n";
+OS << "BUILTIN(__builtin_arm_mve_" << Name << ", \"vi.\", \"nt";
+if (Int.nonEvaluating())
+ OS << "u"; // indicate that this builtin doesn't evaluate its args
+OS << "\")\n";
ShortNamesSeen.insert(Name);
}
}
Index: clang/test/CodeGen/arm-mve-intrinsics/admin.c
===
--- /dev/null
+++ clang/test/CodeGen/arm-mve-intrinsics/admin.c
@@ -0,0 +1,1556 @@
+// NOTE:
[PATCH] D70133: [ARM, MVE] Add intrinsics for 'administrative' vector operations.
simon_tatham marked 3 inline comments as done.
simon_tatham added inline comments.
Comment at: clang/include/clang/Basic/arm_mve.td:384
+ let params = !foldl([], T.All, tlist, srctype, !listconcat(tlist,
+ !if(!eq(!cast(desttype),!cast(srctype)),[],[srctype]))) in {
+def "vreinterpretq_" # desttype: Intrinsic<
dmgreen wrote:
> Indent this line by an extra 4 spaces.
>
> I'm going to have to trust you that it's doing the right thing. The !eq has
> to be on two strings?
Yes, unfortunately – it would be nice to be able to `!eq` two defs, but it only
accepts strings.
Comment at: clang/test/CodeGen/arm-mve-intrinsics/admin.c:1568
+// CHECK-NEXT: entry:
+// CHECK-NEXT:[[TMP0:%.*]] = zext i16 [[A:%.*]] to i32
+// CHECK-NEXT:[[TMP1:%.*]] = insertelement <8 x i16> [[B:%.*]], i16 [[A]],
i32 6
dmgreen wrote:
> What's this zext about? Is it just left over from code that was removed?
`MveEmitter` automatically promotes smaller integer types to `i32`, on the
assumption that you were probably going to pass them to IR intrinsics which
will instruction-select into things taking a GPR as input.
For my next batch of intrinsics I'm working on a system for suppressing that
promotion in cases where it isn't what you wanted after all. Now you mention
it, that might be a thing to pull out and apply here as well.
Comment at: clang/test/CodeGen/arm-mve-intrinsics/admin.c:1671
+//
+float16x8_t test_vuninitializedq_polymorphic_f16(float16x8_t (*funcptr)(void))
+{
dmgreen wrote:
> I see. This is the "non-evaluating" part. It looks odd from a C perspective.
Slightly, although it's no more odd than `sizeof` or `__typeof` for only using
the type of its argument and not actually evaluating it. But it's what ACLE
specifies!
(I think, rather like I just mentioned in D70088, that the use case is for
polymorphism within something like a C++ template – if you have the vector type
as a template parameter, this allows you to generate an uninit value of the
same vector type reasonably easily, without having to write your own system of
C++ template specializations that select the right explicitly suffixed
intrinsic.)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70133/new/
https://reviews.llvm.org/D70133
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70133: [ARM, MVE] Add intrinsics for 'administrative' vector operations.
dmgreen added a comment.
Smaller patches are easier to review, if for example this could have the
vgetq_lane/vsetq_lane split out.
Comment at: clang/include/clang/Basic/arm_mve.td:384
+ let params = !foldl([], T.All, tlist, srctype, !listconcat(tlist,
+ !if(!eq(!cast(desttype),!cast(srctype)),[],[srctype]))) in {
+def "vreinterpretq_" # desttype: Intrinsic<
Indent this line by an extra 4 spaces.
I'm going to have to trust you that it's doing the right thing. The !eq has to
be on two strings?
Comment at: clang/test/CodeGen/arm-mve-intrinsics/admin.c:1568
+// CHECK-NEXT: entry:
+// CHECK-NEXT:[[TMP0:%.*]] = zext i16 [[A:%.*]] to i32
+// CHECK-NEXT:[[TMP1:%.*]] = insertelement <8 x i16> [[B:%.*]], i16 [[A]],
i32 6
What's this zext about? Is it just left over from code that was removed?
Comment at: clang/test/CodeGen/arm-mve-intrinsics/admin.c:1671
+//
+float16x8_t test_vuninitializedq_polymorphic_f16(float16x8_t (*funcptr)(void))
+{
I see. This is the "non-evaluating" part. It looks odd from a C perspective.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70133/new/
https://reviews.llvm.org/D70133
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70133: [ARM, MVE] Add intrinsics for 'administrative' vector operations.
simon_tatham created this revision.
simon_tatham added reviewers: ostannard, MarkMurrayARM, dmgreen.
Herald added subscribers: cfe-commits, kristof.beyls.
Herald added a project: clang.
simon_tatham added a parent revision: D70088: [ARM,MVE] Add intrinsics for
contiguous load/stores..
This batch of intrinsics includes lots of things that move vector data
around or change its type without really affecting its value very
much. It includes the `vreinterpretq` family (cast one vector type to
another); `vuninitializedq` (create a vector of a given type with
don't-care contents); `vcreateq` (make a 128-bit vector out of two
`uint64_t` halves); and the `vgetq_lane` and `vsetq_lane` families, to
read and write an individual lane of a vector.
These are all implemented using completely standard IR that's already
tested in existing LLVM unit tests, so I've just written a clang test
to check the IR is correct, and left it at that.
One of the new `vgetq_lane` intrinsics returns a `float16_t`, which
causes a compile error if `%clang_cc1` doesn't get the option
`-fallow-half-arguments-and-returns`. The driver passes that option to
cc1 already, but I've had to edit all the explicit cc1 command lines
in the existing MVE intrinsics tests.
I've also added some richer infrastructure to the MveEmitter Tablegen
backend, to make it specify the exact integer type of integer
arguments passed to IR construction functions, and wrap those
arguments in a `static_cast` in the autogenerated C++. That was
necessary to prevent an overloading ambiguity when passing the integer
literal `0` to `IRBuilder::CreateInsertElement`, because otherwise, it
could mean either a null pointer `llvm::Value *` or a zero `uint64_t`.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D70133
Files:
clang/include/clang/Basic/arm_mve.td
clang/include/clang/Basic/arm_mve_defs.td
clang/test/CodeGen/arm-mve-intrinsics/admin.c
clang/test/CodeGen/arm-mve-intrinsics/load-store.c
clang/test/CodeGen/arm-mve-intrinsics/scalar-shifts.c
clang/test/CodeGen/arm-mve-intrinsics/scatter-gather.c
clang/test/CodeGen/arm-mve-intrinsics/vadc.c
clang/test/CodeGen/arm-mve-intrinsics/vaddq.c
clang/test/CodeGen/arm-mve-intrinsics/vcvt.c
clang/test/CodeGen/arm-mve-intrinsics/vld24.c
clang/test/CodeGen/arm-mve-intrinsics/vldr.c
clang/test/CodeGen/arm-mve-intrinsics/vminvq.c
clang/test/Sema/arm-mve-immediates.c
clang/utils/TableGen/MveEmitter.cpp
Index: clang/utils/TableGen/MveEmitter.cpp
===
--- clang/utils/TableGen/MveEmitter.cpp
+++ clang/utils/TableGen/MveEmitter.cpp
@@ -632,10 +632,10 @@
StringRef CallPrefix;
std::vector Args;
std::set AddressArgs;
- std::set IntConstantArgs;
+ std::map IntConstantArgs;
IRBuilderResult(StringRef CallPrefix, std::vector Args,
std::set AddressArgs,
- std::set IntConstantArgs)
+ std::map IntConstantArgs)
: CallPrefix(CallPrefix), Args(Args), AddressArgs(AddressArgs),
IntConstantArgs(IntConstantArgs) {}
void genCode(raw_ostream ,
@@ -644,11 +644,13 @@
const char *Sep = "";
for (unsigned i = 0, e = Args.size(); i < e; ++i) {
Ptr Arg = Args[i];
- if (IntConstantArgs.find(i) != IntConstantArgs.end()) {
+ auto it = IntConstantArgs.find(i);
+ if (it != IntConstantArgs.end()) {
assert(Arg->hasIntegerConstantValue());
-OS << Sep
+OS << Sep << "static_cast<" << it->second << ">("
<< ParamAlloc.allocParam("unsigned",
-utostr(Arg->integerConstantValue()));
+utostr(Arg->integerConstantValue()))
+ << ")";
} else {
OS << Sep << Arg->varname();
}
@@ -763,6 +765,14 @@
// shares with at least one other intrinsic.
std::string ShortName, FullName;
+ // A very small number of intrinsics _only_ have a polymorphic
+ // variant (vuninitializedq taking an unevaluated argument).
+ bool PolymorphicOnly;
+
+ // Another rarely-used flag indicating that the builtin doesn't
+ // evaluate its argument(s) at all.
+ bool NonEvaluating;
+
const Type *ReturnType;
std::vector ArgTypes;
std::map ImmediateArgs;
@@ -796,6 +806,8 @@
return false;
}
bool polymorphic() const { return ShortName != FullName; }
+ bool polymorphicOnly() const { return PolymorphicOnly; }
+ bool nonEvaluating() const { return NonEvaluating; }
// External entry point for code generation, called from MveEmitter.
void genCode(raw_ostream , CodeGenParamAllocator ,
@@ -1126,11 +1138,15 @@
Args.push_back(getCodeForDagArg(D, i, Scope, Param));
if (Op->isSubClassOf("IRBuilderBase")) {
std::set AddressArgs;
- for (unsigned i : Op->getValueAsListOfInts("address_params"))
-AddressArgs.insert(i);
- std::set IntConstantArgs;
- for (unsigned i :
