bazelbuild / platforms

Constraint values for specifying platforms and toolchains
Apache License 2.0
108 stars 71 forks source link

add mips64 cpu #24

Closed zunley closed 3 years ago

zunley commented 3 years ago

add mips64 cpu constraints value

zunley commented 3 years ago

@aiuto thanks replay!

Actually, we have finished compiler bazel from scratch on mips64el and ready to submit PRs to bazel after we can add mips64 constrant value to platform.

In terms of architecture naming, mips is very similar to x86, mips32 contains mips32 and mips32el, mips64 contains mips64 and mips64el. It is easy to understand through our adaptation code.

diff --git a/src/main/java/com/google/devtools/build/lib/util/CPU.java b/src/main/java/com/google/devtools/build/lib/util/CPU.java
index 075c94a..3d9c24b 100755
--- a/src/main/java/com/google/devtools/build/lib/util/CPU.java
+++ b/src/main/java/com/google/devtools/build/lib/util/CPU.java
@@ -27,6 +27,7 @@ public enum CPU {
   ARM("arm", ImmutableSet.of("arm", "armv7l")),
   AARCH64("aarch64", ImmutableSet.of("aarch64")),
   S390X("s390x", ImmutableSet.of("s390x", "s390")),
+  MIPS64("mips64",ImmutableSet.of("mips64","mips64el")),
   UNKNOWN("unknown", ImmutableSet.<String>of());

As far as I know, no one uses mips32 anymore, i can't even find mips32 products, but it is certain that we will maintain mips64.

gregestren commented 3 years ago

Thanks for this contribution, @shanjiantao .

I agree with @aiuto that we, as a combined reviewer / contributor community, need to further clarify our expectations of what should and shouldn't go into this repo. With the ultimate goal that it really provides generic, canonical names across projects that maximize interoperability.

We have sketch standards at https://github.com/bazelbuild/platforms#adding-a-canonical-constraint-value. But we need to expand and signal boost it, including new clear examples and counter-examples.

Your particular contribution looks pretty solid to me, especially given your followup explanation. Generally, we want to avoid a future where two targets with mostly similar architectures have totally incompatible selects because the conditions are so fine-grained.

zunley commented 3 years ago

@gregestren All right, i think i hava understood what you mean. Ensuring clarity and simplicity of constraints is essential to the development of Bazel, so the plan to add a new architecture is very import.

As for the mips i have explained, i hope you guys can contact me again after you have a suitable plan, because it is really important for our followup work, thanks!

gregestren commented 3 years ago

We chatted some more and are happy to approve this specific request. Thanks for your further input and patience.

zunley commented 3 years ago

Hi @gregestren, thank you for you efforts.

@aiuto It looks like you need your help to merge. Thanks!

aiuto commented 3 years ago

Whump, whump. The cla bot seems wedged up.

aiuto commented 3 years ago

Oh. Now I see. The auto-import did something and unblocked. I strongly believe we should NOT auto-import this into Google. It should be a manual copybara run to pull from github when we want to sync.