rengolin added subscribers: labrinea, bsmith.
rengolin added a comment.
Hi,
I think it's clear now to me that this strategy isn't going to work. What you
need is to add support for AArch64 in the TargetParser and simplify the name
matching in the same way we did for ARM. There really is no other meaningful
way of doing this.
Please check with Bradley and Alexandros, as they were the ones dealing with
this last.
cheers,
--renato
================
Comment at: lib/Driver/Tools.cpp:1026
@@ +1025,3 @@
+/// valid AArch64 cpu.
+static std::pair<std::vector<const char *>, bool>
+getAArch64FeaturesFromCPU(StringRef CPU) {
----------------
You don't need the IsValid flag, since if the returned vector is empty, it has
the same semantics.
Also, you're returning the vector by value, whereas the pattern used in this
file is to pass a reference to the vector as an argument.
================
Comment at: lib/Driver/Tools.cpp:1042
@@ +1041,3 @@
+
+ return std::make_pair(Features, IsValid);
+}
----------------
You don't need to return a pair. Pass the vector by reference.
You may return a boolean for the IsValid, if there is a case where an empty
vector is valid (no default properties). But as it stands, you don't really
need to.
================
Comment at: lib/Driver/Tools.cpp:1047
@@ +1046,3 @@
+/// targeting. Return false to indicate the cpu is not a valid AArch64 cpu.
+static std::pair<std::string, bool>
+getAArch64TargetCPU(const ArgList &Args) {
----------------
Avoid using pairs as return type. This is really meant for extremely connected
concepts, not to make C++ look like Perl.
================
Comment at: lib/Driver/Tools.cpp:1073
@@ +1072,3 @@
+ IsNative = true;
+ CPU = llvm::sys::getHostCPUName();
+ }
----------------
You could have used a different variable and avoided the ternary operator below.
================
Comment at: lib/Driver/Tools.cpp:1077
@@ +1076,3 @@
+ // Check if CPU is a valid aarch64 cpu.
+ if (getAArch64FeaturesFromCPU(CPU).second)
+ return std::make_pair(CPU, true);
----------------
That's a misuse of this function. You're building a vector, filling it with
stuff, copying it back to a temp and throwing it away just for the CPU name
detection.
This is clearly a job for the TargetParser.
http://reviews.llvm.org/D14471
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
