-
Notifications
You must be signed in to change notification settings - Fork 11.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
clang driver: enable fast unaligned access for Android on RISCV64 #85704
Conversation
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: AdityaK (hiraditya) ChangesAndroid CTS test already requires fast unaligned access https://android-review.googlesource.com/c/platform/cts/+/2675633 Pending testcase Full diff: https://github.com/llvm/llvm-project/pull/85704.diff 2 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
index 5165bccc6d7e30..b1dd7c4372d475 100644
--- a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -167,6 +167,10 @@ void riscv::getRISCVTargetFeatures(const Driver &D, const llvm::Triple &Triple,
Features.push_back("-relax");
}
+ // Android requires fast unaligned access on RISCV64.
+ if (Triple.isAndroid())
+ Features.push_back("+fast-unaligned-access");
+
// -mstrict-align is default, unless -mno-strict-align is specified.
AddTargetFeature(Args, Features, options::OPT_mno_strict_align,
options::OPT_mstrict_align, "fast-unaligned-access");
diff --git a/clang/test/Driver/riscv-features.c b/clang/test/Driver/riscv-features.c
index fe74ac773ef8ca..7db3adb117ae47 100644
--- a/clang/test/Driver/riscv-features.c
+++ b/clang/test/Driver/riscv-features.c
@@ -1,7 +1,7 @@
// RUN: %clang --target=riscv32-unknown-elf -### %s -fsyntax-only 2>&1 | FileCheck %s
// RUN: %clang --target=riscv64-unknown-elf -### %s -fsyntax-only 2>&1 | FileCheck %s
// RUN: %clang --target=riscv64-linux-android -### %s -fsyntax-only 2>&1 | FileCheck %s -check-prefixes=ANDROID,DEFAULT
-// RUN: %clang -mabi=lp64d --target=riscv64-linux-android -### %s -fsyntax-only 2>&1 | FileCheck %s -check-prefixes=ANDROID,DEFAULT
+// RUN: %clang -mabi=lp64d --target=riscv64-linux-android -### %s -fsyntax-only 2>&1 | FileCheck %s -check-prefixes=ANDROID,DEFAULT,FAST-UNALIGNED-ACCESS
// CHECK: fno-signed-char
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
@@ -167,6 +167,10 @@ void riscv::getRISCVTargetFeatures(const Driver &D, const llvm::Triple &Triple, | |||
Features.push_back("-relax"); | |||
} | |||
|
|||
// Android requires fast unaligned access on RISCV64. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This adds two +fast-unaligned-access if -mno-strict-align is specified. Best to update the following AddTargetFeature in case -mstrict-align is specified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
./bin/clang -mabi=lp64d --target=riscv64-linux-android -### -fsyntax-only ../clang/test/Driver/riscv-features.c -mno-strict-align
prints only one "-target-feature" "+fast-unaligned-access"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AIUI, it seems common to add target-features as requested and then at the end tools::unifyTargetFeatures
normalizes the flags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the concern is that -target riscv64-linux-android -mstrict-align
will add +fast-unaligned-access
and -fast-unaligned-access
- which would not be handled correctly by unifyTargetFeatures
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a test to check that following -mstrict-align
would disable fast unaligned access.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
5c8b656
to
0925f52
Compare
Android CTS test already requires fast unaligned access https://android-review.googlesource.com/c/platform/cts/+/2675633
…vm#85704) Android CTS test already requires fast unaligned access https://android-review.googlesource.com/c/platform/cts/+/2675633
Android CTS test already requires fast unaligned access https://android-review.googlesource.com/c/platform/cts/+/2675633