[RISCV] Support predefined macro __riscv_misaligned_[fast,avoid]. #65756

yetingk · 2023-09-08T13:33:38Z

RISC-V C API introduced predefined macro to achieve hints about unaligned accesses (pr). This patch defines __riscv_misaligned_fast when using -mno-strict-align, otherwise, defines __riscv_misaligned_avoid.

Note: This ignores __riscv_misaligned_slow which is also defined by spec.

MaskRay · 2023-09-08T16:41:43Z

clang/test/Preprocessor/riscv-target-features.c

@@ -1220,3 +1220,15 @@
 // RUN: -march=rv64i_zve32x_zvkt1p0 -x c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-ZVKT-EXT %s
 // CHECK-ZVKT-EXT: __riscv_zvkt 1000000{{$}}
+
+// RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32i -x c -E -dM %s \


Use --target= for new tests. -target has been deprecated since 3.4.

Prefer %clang_cc1 for tests outside of test/Driver.

To be fair, the tests in this file are currently %clang. I would argue we should be consistent, which either means %clang here or someone converting the others to %clang_cc1 (so long as those still make sense as cc1 tests). Ditto for -target vs --target=.

Should we update this test file to use --target= for all test cases in another patch? Getting a review comment for copy and pasting from an existing test case is frustrating.

Should we update this test file to use --target= for all test cases in another patch?

Good idea. I think I can create a pr for this.

Should we update this test file to use --target= for all test cases in another patch?

I created a pr #66572 for this.

After #66572 merged, I have rebased the patch on this.

jrtc27 · 2023-09-08T16:44:47Z

clang/lib/Basic/Targets/RISCV.cpp

@@ -322,6 +327,8 @@ bool RISCVTargetInfo::handleTargetFeatures(std::vector<std::string> &Features,
  if (ISAInfo->hasExtension("zfh") || ISAInfo->hasExtension("zhinx"))
    HasLegalHalfType = true;

+  FastUnalignedAccess = llvm::is_contained(Features, "+unaligned-scalar-mem");


What about --target-features +unaligned-scalar-mem --target-features -unaligned-scalar-mem? Unless this has been canonicalised you don't know it's not overridden.

I think clang will does feature canonicalization before calling handleTargetFeatures. It's the piece of code about that in clang/lib/Basic/Targets.cpp.

for (const auto &F : Opts->FeatureMap) Opts->Features.push_back((F.getValue() ? "+" : "-") + F.getKey().str()); // Sort here, so we handle the features in a predictable order. (This matters // when we're dealing with features that overlap.) llvm::sort(Opts->Features); if (!Target->handleTargetFeatures(Opts->Features, Diags)) return nullptr;

yetingk · 2023-09-15T12:59:44Z

Ping.

RISC-V C API introduce predefined macro to achieve hints about unaligned accesses [0]. This defines __riscv_misaligned_fast when using -mno-strict-align, otherwise, defines __riscv_misaligned_avoid. Note: This ignores __riscv_misaligned_slow which is also defined by spec. The spec has mentioned riscv-non-isa/riscv-c-api-doc#40 [0]: riscv-non-isa/riscv-c-api-doc#40

llvmbot · 2023-09-18T08:02:43Z

@llvm/pr-subscribers-backend-risc-v

@llvm/pr-subscribers-clang

Changes

RISC-V C API introduced predefined macro to achieve hints about unaligned accesses (pr). This patch defines __riscv_misaligned_fast when using -mno-strict-align, otherwise, defines __riscv_misaligned_avoid.

Note: This ignores __riscv_misaligned_slow which is also defined by spec.

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

3 Files Affected:

(modified) clang/lib/Basic/Targets/RISCV.cpp (+7)
(modified) clang/lib/Basic/Targets/RISCV.h (+3)
(modified) clang/test/Preprocessor/riscv-target-features.c (+12)

diff --git a/clang/lib/Basic/Targets/RISCV.cpp b/clang/lib/Basic/Targets/RISCV.cpp
index d55ab76395c8271..119d905be57249b 100644
--- a/clang/lib/Basic/Targets/RISCV.cpp
+++ b/clang/lib/Basic/Targets/RISCV.cpp
@@ -204,6 +204,11 @@ void RISCVTargetInfo::getTargetDefines(const LangOptions &Opts,
   if (VScale && VScale->first && VScale->first == VScale->second)
     Builder.defineMacro("__riscv_v_fixed_vlen",
                         Twine(VScale->first * llvm::RISCV::RVVBitsPerBlock));
+
+  if (FastUnalignedAccess)
+    Builder.defineMacro("__riscv_misaligned_fast");
+  else
+    Builder.defineMacro("__riscv_misaligned_avoid");
 }
 
 static constexpr Builtin::Info BuiltinInfo[] = {
@@ -322,6 +327,8 @@ bool RISCVTargetInfo::handleTargetFeatures(std::vector<std::string> &Features,
   if (ISAInfo->hasExtension("zfh") || ISAInfo->hasExtension("zhinx"))
     HasLegalHalfType = true;
 
+  FastUnalignedAccess = llvm::is_contained(Features, "+unaligned-scalar-mem");
+
   return true;
 }
 
diff --git a/clang/lib/Basic/Targets/RISCV.h b/clang/lib/Basic/Targets/RISCV.h
index 6be0e49ca2f5525..e5424d318401fb0 100644
--- a/clang/lib/Basic/Targets/RISCV.h
+++ b/clang/lib/Basic/Targets/RISCV.h
@@ -29,6 +29,9 @@ class RISCVTargetInfo : public TargetInfo {
   std::string ABI, CPU;
   std::unique_ptr<llvm::RISCVISAInfo> ISAInfo;
 
+private:
+  bool FastUnalignedAccess;
+
 public:
   RISCVTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
       : TargetInfo(Triple) {
diff --git a/clang/test/Preprocessor/riscv-target-features.c b/clang/test/Preprocessor/riscv-target-features.c
index 4dd83cfa0620b90..4db1cf7a49272fb 100644
--- a/clang/test/Preprocessor/riscv-target-features.c
+++ b/clang/test/Preprocessor/riscv-target-features.c
@@ -1228,3 +1228,15 @@
 // RUN: -march=rv64i_zve32x_zvkt1p0 -x c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-ZVKT-EXT %s
 // CHECK-ZVKT-EXT: __riscv_zvkt 1000000{{$}}
+
+// RUN: %clang --target=riscv32-unknown-linux-gnu -march=rv32i -x c -E -dM %s \
+// RUN: -o - | FileCheck %s --check-prefix=CHECK-MISALIGNED-AVOID
+// RUN: %clang --target=riscv64-unknown-linux-gnu -march=rv64i -x c -E -dM %s \
+// RUN: -o - | FileCheck %s --check-prefix=CHECK-MISALIGNED-AVOID
+// CHECK-MISALIGNED-AVOID: __riscv_misaligned_avoid 1
+
+// RUN: %clang --target=riscv32-unknown-linux-gnu -march=rv32i -x c -E -dM %s \
+// RUN: -munaligned-access -o - | FileCheck %s --check-prefix=CHECK-MISALIGNED-FAST
+// RUN: %clang --target=riscv64-unknown-linux-gnu -march=rv64i -x c -E -dM %s \
+// RUN: -munaligned-access -o - | FileCheck %s --check-prefix=CHECK-MISALIGNED-FAST
+// CHECK-MISALIGNED-FAST: __riscv_misaligned_fast 1

yetingk · 2023-10-17T07:02:07Z

Ping.

topperc

LGTM

MaskRay · 2023-10-26T03:14:30Z

clang/test/Preprocessor/riscv-target-features.c

@@ -1228,3 +1228,15 @@
 // RUN: -march=rv64i_zve32x_zvkt1p0 -x c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-ZVKT-EXT %s
 // CHECK-ZVKT-EXT: __riscv_zvkt 1000000{{$}}
+
+// RUN: %clang --target=riscv32-unknown-linux-gnu -march=rv32i -x c -E -dM %s \


Remove -x c (redundant) and indent the continuation lines by 2

MaskRay · 2023-10-26T03:15:28Z

[RISCV] Support predefined marcro _riscv_misaligned{fast,avoid}.

The majority of patches don't add a trailing period in the title.

…vm#65756) RISC-V C API introduced predefined macro to achieve hints about unaligned accesses ([pr]). This patch defines __riscv_misaligned_fast when using -mno-strict-align, otherwise, defines __riscv_misaligned_avoid. Note: This ignores __riscv_misaligned_slow which is also defined by spec. [pr]: riscv-non-isa/riscv-c-api-doc#40

yetingk requested a review from a team as a code owner September 8, 2023 13:33

github-actions bot added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label Sep 8, 2023

MaskRay reviewed Sep 8, 2023

View reviewed changes

jrtc27 reviewed Sep 8, 2023

View reviewed changes

yetingk force-pushed the misaligned branch from 4d5ad30 to 391711d Compare September 18, 2023 08:01

llvmbot added clang Clang issues not falling into any other category backend:RISC-V labels Sep 18, 2023

topperc mentioned this pull request Oct 26, 2023

-march=rv64gc -munaligned-access should define __riscv_misaligned_fast #70275

Closed

topperc approved these changes Oct 26, 2023

View reviewed changes

MaskRay approved these changes Oct 26, 2023

View reviewed changes

yetingk changed the title ~~[RISCV] Support predefined marcro __riscv_misaligned_{fast,avoid}.~~ [RISCV] Support predefined marcro __riscv_misaligned_[fast,avoid]. Oct 26, 2023

topperc changed the title ~~[RISCV] Support predefined marcro __riscv_misaligned_[fast,avoid].~~ [RISCV] Support predefined macro __riscv_misaligned_[fast,avoid]. Oct 26, 2023

[RISCV] Refine riscv-target-features.c

04600b8

yetingk merged commit 6e2d67e into llvm:main Oct 26, 2023
1 of 3 checks passed

tomeksowi mentioned this pull request Feb 19, 2024

[RISC-V] Remove unnecessary assertion from emitOutputInstr dotnet/runtime#98484

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RISCV] Support predefined macro __riscv_misaligned_[fast,avoid]. #65756

[RISCV] Support predefined macro __riscv_misaligned_[fast,avoid]. #65756

yetingk commented Sep 8, 2023 •

edited

Loading

MaskRay Sep 8, 2023

jrtc27 Sep 8, 2023 •

edited

Loading

topperc Sep 15, 2023

yetingk Sep 16, 2023 •

edited

Loading

yetingk Sep 16, 2023 •

edited

Loading

yetingk Sep 18, 2023

jrtc27 Sep 8, 2023

yetingk Sep 9, 2023

yetingk commented Sep 15, 2023

llvmbot commented Sep 18, 2023 •

edited

Loading

yetingk commented Oct 17, 2023

topperc left a comment

MaskRay Oct 26, 2023

yetingk Oct 26, 2023

MaskRay commented Oct 26, 2023

[RISCV] Support predefined macro __riscv_misaligned_[fast,avoid]. #65756

[RISCV] Support predefined macro __riscv_misaligned_[fast,avoid]. #65756

Conversation

yetingk commented Sep 8, 2023 • edited Loading

MaskRay Sep 8, 2023

Choose a reason for hiding this comment

jrtc27 Sep 8, 2023 • edited Loading

Choose a reason for hiding this comment

topperc Sep 15, 2023

Choose a reason for hiding this comment

yetingk Sep 16, 2023 • edited Loading

Choose a reason for hiding this comment

yetingk Sep 16, 2023 • edited Loading

Choose a reason for hiding this comment

yetingk Sep 18, 2023

Choose a reason for hiding this comment

jrtc27 Sep 8, 2023

Choose a reason for hiding this comment

yetingk Sep 9, 2023

Choose a reason for hiding this comment

yetingk commented Sep 15, 2023

llvmbot commented Sep 18, 2023 • edited Loading

yetingk commented Oct 17, 2023

topperc left a comment

Choose a reason for hiding this comment

MaskRay Oct 26, 2023

Choose a reason for hiding this comment

yetingk Oct 26, 2023

Choose a reason for hiding this comment

MaskRay commented Oct 26, 2023

yetingk commented Sep 8, 2023 •

edited

Loading

jrtc27 Sep 8, 2023 •

edited

Loading

yetingk Sep 16, 2023 •

edited

Loading

yetingk Sep 16, 2023 •

edited

Loading

llvmbot commented Sep 18, 2023 •

edited

Loading