[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