[PATCH] D130255: [Clang][LoongArch] Add initial LoongArch target and driver support

[Renato Golin via Phabricator via cfe-commits]

rengolin added a comment.

This looks great, thanks!

Exciting that we can finally go all the way from C source to LoongArch binary.

The changes look good to me, other than a few nits. But please wait for a day or two for other people to review on their own time.



================
Comment at: clang/lib/Basic/Targets/LoongArch.h:69
+    // TODO: select appropriate ABI.
+    ABI = "ilp32d";
+  }
----------------
nit: this should use `setABI`. Right now, there's no difference, but once the function does more (like setting other ABI flags), you won't need to change it here. Same for the 64-bit version.


================
Comment at: clang/lib/Driver/ToolChains/Arch/LoongArch.h:25
+} // end namespace loongarch
+} // namespace tools
+} // end namespace driver
----------------
nit: comment mismatch "end"


================
Comment at: clang/test/Driver/loongarch-abi.c:41
+
+// CHECK-LA32-LP64S: error: unknown target ABI 'lp64s'
+// CHECK-LA32-LP64F: error: unknown target ABI 'lp64f'
----------------
please, split between pass and error by adding a new `loongarch-abi-error.c` and adding the `error` tests to it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130255