[PATCH] D115857: [LoongArch 1/5] Add triples loongarch{32, 64} for the upcoming LoongArch target

[Renato Golin via Phabricator via llvm-commits]

rengolin added inline comments.


================
Comment at: llvm/unittests/ADT/TripleTest.cpp:357
 
+  T = Triple("loongarch32-unknown-unknown");
+  EXPECT_EQ(Triple::loongarch32, T.getArch());
----------------
SixWeining wrote:
> rengolin wrote:
> > I'm assuming this is bare metal?
> > 
> > If you haven't fixed this elsewhere, you may want to do it now, for example, using the name of your ABI like Arm does (aeabi). 
> > 
> > But this is totally optional, because it doesn't make much of a difference if you only ever have one ABI for bare metal.
> "loongarch32-unknown-unknown" is only a basic triple test for the loongarch32 target and here we just follow what other architectures do, like csky (line 345), riscv(line 369).
> 
> Currently GCC is proposing using below triples
> loongarch64-linux-gnuf64
> loongarch64-linux-gnuf32
> loongarch64-linux-gnusf
> loongarch32-linux-gnuf64
> loongarch32-linux-gnuf32
> loongarch32-linux-gnusf
> 
> I think we have 2 options:
> 1. Delete this triple test for loongarch32 and add it/them back until the environment names are upstreamed to GCC.
> 1. no change.
It's your target, you get to choose what the triples are.

GCC probably wants to put "gnu" as the environment because that's what they provide, so they're not forced to conform to ill-specified external ABIs (not your case).

If you have a specific ABI name and has issues with it being called "gnu" I think you can make the case to the GCC folks, but it usually isn't necessary unless it's a certification (or a marketing) issue.

We don't really care what the targets are called, as long as they're used in the wild, which generally means GCC. So if you're happy with those triples, we're happy with them too.

Regarding the two-step process, it doesn't matter. `unknwon` is a perfectly valid triple component, so feel free to leave it there, even after you add more triples.

My comment was just for clarification.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115857/new/

https://reviews.llvm.org/D115857