[PATCH] D40985: [AArch64] Add Exynos to host detection
Zachary Turner via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 7 14:27:35 PST 2017
zturner added inline comments.
================
Comment at: llvm/lib/Support/Host.cpp:222
+ // any predictive pattern across variants and parts.
+ unsigned Variant = 0, Part = 0, Exynos;
+
----------------
Can you initialize `Exynos` to some value here?
================
Comment at: llvm/lib/Support/Host.cpp:233-234
+ for (auto I : Lines)
+ if (I.startswith("CPU part"))
+ I.substr(8).ltrim("\t :").getAsInteger(16, Part);
+
----------------
How about
```
if (I.consume_front("CPU part"))
I.ltrim("\t :").getAsInteger(16, Part);
```
As an example of why I prefer `consume_front`, it fixes what appears to be above in the preceding if statement (about CPU variant), where you've hardcoded 8 but actually meant 11.
================
Comment at: llvm/lib/Support/Host.cpp:236
+
+ Exynos = (Variant << 12) + Part;
+ switch (Exynos) {
----------------
Can you move the declaration of `Exynos` down here and just say:
`unsigned Exynos = (Variant << 12) + Part;`
That also simultaneously addresses my previous comment about initialization.
Repository:
rL LLVM
https://reviews.llvm.org/D40985
More information about the llvm-commits
mailing list