[clang][LoongArch] Add OHOS target #127555

Ami-zhang · 2025-02-18T02:30:12Z

Add support for OHOS on loongarch64.

llvmbot · 2025-02-18T02:30:49Z

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: None (Ami-zhang)

Changes

Add support for OHOS on LoongArch.

This patch is taken from part of the
https://gitee.com/openharmony/third_party_llvm-project/pulls/554, the original author is @caiwei.

Full diff: https://github.com/llvm/llvm-project/pull/127555.diff

4 Files Affected:

(modified) clang/lib/Basic/Targets.cpp (+20-4)
(modified) clang/lib/Driver/ToolChains/Gnu.cpp (+2-1)
(modified) clang/lib/Driver/ToolChains/OHOS.cpp (+12-1)
(modified) clang/test/Preprocessor/ohos.c (+2)

diff --git a/clang/lib/Basic/Targets.cpp b/clang/lib/Basic/Targets.cpp
index 281aebdb1c35d..f345e3596df39 100644
--- a/clang/lib/Basic/Targets.cpp
+++ b/clang/lib/Basic/Targets.cpp
@@ -741,16 +741,32 @@ std::unique_ptr<TargetInfo> AllocateTarget(const llvm::Triple &Triple,
   case llvm::Triple::loongarch32:
     switch (os) {
     case llvm::Triple::Linux:
-      return std::make_unique<LinuxTargetInfo<LoongArch32TargetInfo>>(Triple,
-                                                                      Opts);
+      // OHOS_LOCAL begin
+      switch (Triple.getEnvironment()) {
+      default:
+        return std::make_unique<LinuxTargetInfo<LoongArch32TargetInfo>>(Triple,
+                                                                        Opts);
+      case llvm::Triple::OpenHOS:
+        return std::make_unique<OHOSTargetInfo<LoongArch32TargetInfo>>(Triple,
+                                                                       Opts);
+      }
+      // OHOS_LOCAL end
     default:
       return std::make_unique<LoongArch32TargetInfo>(Triple, Opts);
     }
   case llvm::Triple::loongarch64:
     switch (os) {
     case llvm::Triple::Linux:
-      return std::make_unique<LinuxTargetInfo<LoongArch64TargetInfo>>(Triple,
-                                                                      Opts);
+      // OHOS_LOCAL begin
+      switch (Triple.getEnvironment()) {
+      default:
+        return std::make_unique<LinuxTargetInfo<LoongArch64TargetInfo>>(Triple,
+                                                                        Opts);
+      case llvm::Triple::OpenHOS:
+        return std::make_unique<OHOSTargetInfo<LoongArch64TargetInfo>>(Triple,
+                                                                       Opts);
+      }
+      // OHOS_LOCAL end
     case llvm::Triple::FreeBSD:
       return std::make_unique<FreeBSDTargetInfo<LoongArch64TargetInfo>>(Triple,
                                                                         Opts);
diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp
index f56eeda3cb5f6..0b560be1d28e7 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2458,7 +2458,8 @@ void Generic_GCC::GCCInstallationDetector::AddDefaultGCCPrefixes(
 
   static const char *const LoongArch64LibDirs[] = {"/lib64", "/lib"};
   static const char *const LoongArch64Triples[] = {
-      "loongarch64-linux-gnu", "loongarch64-unknown-linux-gnu"};
+      "loongarch64-linux-gnu", "loongarch64-unknown-linux-gnu",
+      "loongarch64-linux-ohos"}; // OHOS_LOCAL
 
   static const char *const M68kLibDirs[] = {"/lib"};
   static const char *const M68kTriples[] = {"m68k-unknown-linux-gnu",
diff --git a/clang/lib/Driver/ToolChains/OHOS.cpp b/clang/lib/Driver/ToolChains/OHOS.cpp
index 6e1a09ae908b2..cd4351a2c614d 100644
--- a/clang/lib/Driver/ToolChains/OHOS.cpp
+++ b/clang/lib/Driver/ToolChains/OHOS.cpp
@@ -111,6 +111,10 @@ std::string OHOS::getMultiarchTriple(const llvm::Triple &T) const {
     return "x86_64-linux-ohos";
   case llvm::Triple::aarch64:
     return "aarch64-linux-ohos";
+  // OHOS_LOCAL begin
+  case llvm::Triple::loongarch64:
+    return "loongarch64-linux-ohos";
+    // OHOS_LOCAL end
   }
   return T.str();
 }
@@ -368,7 +372,14 @@ void OHOS::addExtraOpts(llvm::opt::ArgStringList &CmdArgs) const {
   CmdArgs.push_back("-z");
   CmdArgs.push_back("relro");
   CmdArgs.push_back("-z");
-  CmdArgs.push_back("max-page-size=4096");
+  // OHOS_LOCAL begin
+  // LoongArch needs page size 16K
+  if (getArch() == llvm::Triple::loongarch64) {
+    CmdArgs.push_back("max-page-size=16384");
+  } else {
+    CmdArgs.push_back("max-page-size=4096");
+  }
+  // OHOS_LOCAL end
   // .gnu.hash section is not compatible with the MIPS target
   if (getArch() != llvm::Triple::mipsel)
     CmdArgs.push_back("--hash-style=both");
diff --git a/clang/test/Preprocessor/ohos.c b/clang/test/Preprocessor/ohos.c
index 0c435c7ed5ab4..7017c9847ccae 100644
--- a/clang/test/Preprocessor/ohos.c
+++ b/clang/test/Preprocessor/ohos.c
@@ -3,6 +3,7 @@
 // RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -triple=riscv64-linux-ohos < /dev/null | FileCheck %s -match-full-lines -check-prefix=RISCV64-OHOS-CXX
 // RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -triple=mipsel-linux-ohos < /dev/null | FileCheck %s -match-full-lines -check-prefix=MIPSEL-OHOS-CXX
 // RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -triple=x86_64-linux-ohos < /dev/null | FileCheck %s -match-full-lines -check-prefix=X86_64-OHOS-CXX
+// RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -triple=loongarch64-linux-ohos < /dev/null | FileCheck %s -match-full-lines -check-prefix=LOONGARCH64-OHOS-CXX
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=arm-linux-ohos < /dev/null | FileCheck %s -check-prefix=OHOS-DEFS
 
 // ARM-OHOS-CXX: #define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 8U
@@ -10,6 +11,7 @@
 // RISCV64-OHOS-CXX: #define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 16UL
 // MIPSEL-OHOS-CXX: #define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 8U
 // X86_64-OHOS-CXX: #define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 16UL
+// LOONGARCH64-OHOS-CXX: #define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 16UL
 // OHOS-DEFS: __OHOS_FAMILY__
 // OHOS-DEFS: __OHOS__
 // OHOS-DEFS-NOT: __OHOS__

brad0 · 2025-02-18T07:18:34Z

Remove all of the OHOS_LOCAL begin / end comment markers.

brad0 · 2025-02-18T07:24:32Z

You have to push any new changes first.

Ami-zhang · 2025-02-18T07:30:23Z

Remove all of the OHOS_LOCAL begin / end comment markers.

Done.
Thank you for your review.

brad0 · 2025-02-18T07:32:41Z

That looks better.

brad0 · 2025-02-18T07:43:59Z

clang/test/Preprocessor/ohos.c

 // RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -triple=riscv64-linux-ohos < /dev/null | FileCheck %s -match-full-lines -check-prefix=RISCV64-OHOS-CXX
 // RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -triple=mipsel-linux-ohos < /dev/null | FileCheck %s -match-full-lines -check-prefix=MIPSEL-OHOS-CXX
 // RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -triple=x86_64-linux-ohos < /dev/null | FileCheck %s -match-full-lines -check-prefix=X86_64-OHOS-CXX
+// RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -triple=loongarch64-linux-ohos < /dev/null | FileCheck %s -match-full-lines -check-prefix=LOONGARCH64-OHOS-CXX


Should test loongarch32 here too.

brad0 · 2025-02-18T07:44:01Z

clang/lib/Driver/ToolChains/OHOS.cpp

    return "x86_64-linux-ohos";
  case llvm::Triple::aarch64:
    return "aarch64-linux-ohos";
+  case llvm::Triple::loongarch64:


No loongarch32 addition here?

Yeah.
Just on loongarch64.

SixWeining

LGTM for the LoongArch change. We'll see if @kpdev has any objections later.

SixWeining · 2025-02-19T01:21:53Z

clang/lib/Driver/ToolChains/OHOS.cpp

+  if (getArch() == llvm::Triple::loongarch64) {
+    CmdArgs.push_back("max-page-size=16384");
+  } else {
+    CmdArgs.push_back("max-page-size=4096");
+  }


Suggested change

if (getArch() == llvm::Triple::loongarch64) {

CmdArgs.push_back("max-page-size=16384");

} else {

CmdArgs.push_back("max-page-size=4096");

}

CmdArgs.push_back(getArch() == llvm::Triple::loongarch64

? "max-page-size=16384"

: "max-page-size=4096");

Okay, Thanks.

SixWeining · 2025-02-19T01:22:22Z

clang/lib/Driver/ToolChains/OHOS.cpp

  CmdArgs.push_back("relro");
  CmdArgs.push_back("-z");
-  CmdArgs.push_back("max-page-size=4096");
+  // LoongArch needs page size 16K


The comment is unnecessary as the code explains itself.

SixWeining

I think you should add more tests like clang/test/Driver/loongarch-toolchain.c and https://reviews.llvm.org/D55029.

SixWeining · 2025-02-19T01:31:49Z

clang/lib/Basic/Targets.cpp

+      case llvm::Triple::OpenHOS:
+        return std::make_unique<OHOSTargetInfo<LoongArch32TargetInfo>>(Triple,
+                                                                       Opts);
+      }


I think this patch should only focus on LoongArch64. LoongArch32 could be added in future.

SixWeining · 2025-02-19T01:42:24Z

clang/lib/Driver/ToolChains/Gnu.cpp

  static const char *const LoongArch64Triples[] = {
-      "loongarch64-linux-gnu", "loongarch64-unknown-linux-gnu"};
+      "loongarch64-linux-gnu", "loongarch64-unknown-linux-gnu",
+      "loongarch64-linux-ohos"};


Why the ohos triple is added to Gnu.cpp. Seems that aarch64 and x86-64 don't add them.

Keep it consistent with other architectures and do not add it for now.
Removed.

Please remove new entries. There is a comment to discourage new entries.

MaskRay · 2025-02-19T05:36:43Z

clang/lib/Driver/ToolChains/OHOS.cpp

  CmdArgs.push_back("relro");
  CmdArgs.push_back("-z");
-  CmdArgs.push_back("max-page-size=4096");
+  CmdArgs.push_back(getArch() == llvm::Triple::loongarch64


This should be tested

Added a test in the latest patch. Thanks!

brad0 · 2025-02-19T09:10:04Z

clang/test/Driver/loongarch-ohos-pagesize.c

@@ -0,0 +1,6 @@
+/// Check the max-page-size for OHOS LoongArch.


I'd rename the test like ohos-pagesize.c. Maybe add one other arch and check that the value is 4k.

kpdev · 2025-02-19T19:45:41Z

LGTM for the LoongArch change. We'll see if @kpdev has any objections later.

LGTM also. Thank you for the patch

MaskRay · 2025-02-20T06:28:54Z

clang/test/Driver/ohos-pagesize.c

@@ -0,0 +1,16 @@
+/// Check the max-page-size for OHOS aarch64/loongarch64/riscv64/x86_64.


Don't add a new file for a minor property (though important to test) like page-size. Just reuse ohos.c

Many driver tests are probably not organized in the best way. linux-cross.cpp could give inspiration.
You usually need to rg max-page-size clang/test/Driver, read multiple tests , and decide how to write the test in the most elegant way:)

Removed the new file and added the page-size check in ohos.c, following the style of similar tests. Thanks!

clang/test/Driver/ohos.c

SixWeining · 2025-02-20T09:34:00Z

Could you add some tests in ohos.c with the --sysroot option specified?

Ami-zhang · 2025-02-21T08:41:19Z

Could you add some tests in ohos.c with the --sysroot option specified?

Could I confirm what the test targeting the --sysroot option is for? Is it also intended to test the page-size?

SixWeining · 2025-02-21T08:55:08Z

Could you add some tests in ohos.c with the --sysroot option specified?

Could I confirm what the test targeting the --sysroot option is for? Is it also intended to test the page-size?

I mean the purpose is to check whether clang driver passes correct options to cc1 or lld. For example, the ld-musl-loongarch64.so.1, crti.o, crt1.o, etc.

Ami-zhang · 2025-02-25T03:44:46Z

Could you add some tests in ohos.c with the --sysroot option specified?

Could I confirm what the test targeting the --sysroot option is for? Is it also intended to test the page-size?

I mean the purpose is to check whether clang driver passes correct options to cc1 or lld. For example, the ld-musl-loongarch64.so.1, crti.o, crt1.o, etc.

Okay, the test has been added.

Ami-zhang · 2025-02-27T01:33:22Z

Ping
Does anyone have any further comments?:)

clang/test/Driver/ohos.c

Add support for OHOS on loongarch64.

SixWeining

LGTM

llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Feb 18, 2025

Ami-zhang requested review from SixWeining and wangleiat February 18, 2025 07:08

Ami-zhang requested a review from brad0 February 18, 2025 07:21

Ami-zhang force-pushed the ohos branch from e53bc2f to daf1b4d Compare February 18, 2025 07:25

brad0 reviewed Feb 18, 2025

View reviewed changes

SixWeining reviewed Feb 19, 2025

View reviewed changes

SixWeining requested review from heiher and kpdev February 19, 2025 01:50

Ami-zhang force-pushed the ohos branch from daf1b4d to c6df3aa Compare February 19, 2025 03:25

MaskRay reviewed Feb 19, 2025

View reviewed changes

Ami-zhang force-pushed the ohos branch from c6df3aa to 5a2d1eb Compare February 19, 2025 08:58

brad0 reviewed Feb 19, 2025

View reviewed changes

Ami-zhang force-pushed the ohos branch from 5a2d1eb to 65d913f Compare February 19, 2025 09:29

MaskRay reviewed Feb 20, 2025

View reviewed changes

Ami-zhang force-pushed the ohos branch from 65d913f to e675b4b Compare February 20, 2025 07:56

SixWeining reviewed Feb 20, 2025

View reviewed changes

clang/test/Driver/ohos.c Outdated Show resolved Hide resolved

Ami-zhang force-pushed the ohos branch from e675b4b to f1d1471 Compare February 25, 2025 03:41

SixWeining reviewed Feb 27, 2025

View reviewed changes

clang/test/Driver/ohos.c Show resolved Hide resolved

[clang][LoongArch] Add OHOS target

49312a2

Add support for OHOS on loongarch64.

Ami-zhang force-pushed the ohos branch from f1d1471 to 49312a2 Compare February 27, 2025 07:51

SixWeining approved these changes Feb 27, 2025

View reviewed changes

Ami-zhang merged commit f5f3612 into llvm:main Mar 3, 2025
11 checks passed

Ami-zhang deleted the ohos branch March 3, 2025 01:26

		@@ -0,0 +1,6 @@
		/// Check the max-page-size for OHOS LoongArch.

		@@ -0,0 +1,16 @@
		/// Check the max-page-size for OHOS aarch64/loongarch64/riscv64/x86_64.

[clang][LoongArch] Add OHOS target #127555

[clang][LoongArch] Add OHOS target #127555

Uh oh!

Conversation

Ami-zhang commented Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brad0 commented Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brad0 commented Feb 18, 2025

Uh oh!

Ami-zhang commented Feb 18, 2025

Uh oh!

brad0 commented Feb 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SixWeining left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SixWeining left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kpdev commented Feb 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SixWeining commented Feb 20, 2025

Uh oh!

Ami-zhang commented Feb 21, 2025

Uh oh!

SixWeining commented Feb 21, 2025

Uh oh!

Ami-zhang commented Feb 25, 2025

Uh oh!

Ami-zhang commented Feb 27, 2025

Uh oh!

Uh oh!

SixWeining left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Ami-zhang commented Feb 18, 2025 •

edited

Loading

llvmbot commented Feb 18, 2025 •

edited

Loading

brad0 commented Feb 18, 2025 •

edited

Loading