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

Lu Weining via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 21 19:17:38 PDT 2022


SixWeining added a comment.

In D130255#3668436 <https://reviews.llvm.org/D130255#3668436>, @rengolin wrote:

> 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.

Thanks, Renato.

Because relocation hasn't been added to the backend, complex code (e.g. with function call) can't been compiled currently.
But at least, as you said,  we finally go all the way from C source to LoongArch binary. This is a start and further implementation will be added later.



================
Comment at: clang/lib/Basic/Targets/LoongArch.h:69
+    // TODO: select appropriate ABI.
+    ABI = "ilp32d";
+  }
----------------
rengolin wrote:
> 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.
Thanks. No problem.


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


================
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'
----------------
rengolin wrote:
> please, split between pass and error by adding a new `loongarch-abi-error.c` and adding the `error` tests to it.
Thanks. No problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130255



More information about the cfe-commits mailing list