[DirectX] Fix HLSL bitshifts to leverage the OpenCL pipeline for bitshifting #81030
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write If you have received no comments on your PR for a week, you can request a review If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen Author: Cooper Partin (coopp) ChangesFull diff: https://github.com/llvm/llvm-project/pull/81030.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index df8f71cf1d9008..fa03163bbde577 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -4168,7 +4168,7 @@ Value *ScalarExprEmitter::EmitShl(const BinOpInfo &Ops) {
bool SanitizeBase = SanitizeSignedBase || SanitizeUnsignedBase;
bool SanitizeExponent = CGF.SanOpts.has(SanitizerKind::ShiftExponent);
// OpenCL 6.3j: shift values are effectively % word size of LHS.
- if (CGF.getLangOpts().OpenCL)
+ if (CGF.getLangOpts().OpenCL || CGF.getLangOpts().HLSL)
RHS = ConstrainShiftValue(Ops.LHS, RHS, "shl.mask");
else if ((SanitizeBase || SanitizeExponent) &&
isa<llvm::IntegerType>(Ops.LHS->getType())) {
@@ -4237,7 +4237,7 @@ Value *ScalarExprEmitter::EmitShr(const BinOpInfo &Ops) {
RHS = Builder.CreateIntCast(RHS, Ops.LHS->getType(), false, "sh_prom");
// OpenCL 6.3j: shift values are effectively % word size of LHS.
- if (CGF.getLangOpts().OpenCL)
+ if (CGF.getLangOpts().OpenCL || CGF.getLangOpts().HLSL)
RHS = ConstrainShiftValue(Ops.LHS, RHS, "shr.mask");
else if (CGF.SanOpts.has(SanitizerKind::ShiftExponent) &&
isa<llvm::IntegerType>(Ops.LHS->getType())) {
diff --git a/clang/test/CodeGenHLSL/shift-mask.hlsl b/clang/test/CodeGenHLSL/shift-mask.hlsl
new file mode 100644
index 00000000000000..b8a7dced1a0ee5
--- /dev/null
+++ b/clang/test/CodeGenHLSL/shift-mask.hlsl
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -finclude-default-header -x hlsl -triple \
+// RUN: dxil-pc-shadermodel6.3-library %s \
+// RUN: -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+int shl32(int V, int S) {
+ return V << S;
+}
+
+// CHECK: define noundef i32 @"?shl32{{[@$?.A-Za-z0-9_]+}}"(i32 noundef %V, i32 noundef %S) #0 {
+// CHECK-DAG: %[[Masked:.*]] = and i32 %{{.*}}, 31
+// CHECK-DAG: %{{.*}} = shl i32 %{{.*}}, %[[Masked]]
+
+int shr32(int V, int S) {
+ return V >> S;
+}
+
+// CHECK: define noundef i32 @"?shr32{{[@$?.A-Za-z0-9_]+}}"(i32 noundef %V, i32 noundef %S) #0 {
+// CHECK-DAG: %[[Masked:.*]] = and i32 %{{.*}}, 31
+// CHECK-DAG: %{{.*}} = ashr i32 %{{.*}}, %[[Masked]]
+
+int64_t shl64(int64_t V, int64_t S) {
+ return V << S;
+}
+
+// CHECK: define noundef i64 @"?shl64{{[@$?.A-Za-z0-9_]+}}"(i64 noundef %V, i64 noundef %S) #0 {
+// CHECK-DAG: %[[Masked:.*]] = and i64 %{{.*}}, 63
+// CHECK-DAG: %{{.*}} = shl i64 %{{.*}}, %[[Masked]]
+
+int64_t shr64(int64_t V, int64_t S) {
+ return V >> S;
+}
+
+// CHECK: define noundef i64 @"?shr64{{[@$?.A-Za-z0-9_]+}}"(i64 noundef %V, i64 noundef %S) #0 {
+// CHECK-DAG: %[[Masked:.*]] = and i64 %{{.*}}, 63
+// CHECK-DAG: %{{.*}} = ashr i64 %{{.*}}, %[[Masked]]
\ No newline at end of file
|
coopp
changed the title
|
What is the output before this change? |
coopp
changed the title
coopp
changed the title
I added a description that shows a before/after output for this change. |
|
The bot failure looks like an unrelated CMake issue on Windows. I think we've seen that around a few times. I'll merge this now. |
|
@coopp Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested Please check whether problems have been caused by your change specifically, as How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
Fixes #55106
In HLSL bit shifts are defined to shift by shift size % type size. This contains the following changes:
HLSL codegen bit shifts will be emitted as x << (y & (sizeof(x) - 1) and bitshift masking leverages the OpenCL pipeline for this.
Tests were also added to validate this behavior.
Before this change the following was being emitted:
; Function Attrs: noinline nounwind optnone
define noundef i32 @"?shl32@@yahhh@Z"(i32 noundef %V, i32 noundef %S) #0 {
entry:
%S.addr = alloca i32, align 4
%V.addr = alloca i32, align 4
store i32 %S, ptr %S.addr, align 4
store i32 %V, ptr %V.addr, align 4
%0 = load i32, ptr %V.addr, align 4
%1 = load i32, ptr %S.addr, align 4
%shl = shl i32 %0, %1
ret i32 %shl
}
After this change:
; Function Attrs: noinline nounwind optnone
define noundef i32 @"?shl32@@yahhh@Z"(i32 noundef %V, i32 noundef %S) #0 {
entry:
%S.addr = alloca i32, align 4
%V.addr = alloca i32, align 4
store i32 %S, ptr %S.addr, align 4
store i32 %V, ptr %V.addr, align 4
%0 = load i32, ptr %V.addr, align 4
%1 = load i32, ptr %S.addr, align 4
%shl.mask = and i32 %1, 31
%shl = shl i32 %0, %shl.mask
ret i32 %shl
}