Skip to content
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

[HLSL] Array by-value assignment #109323

Merged
merged 4 commits into from
Oct 1, 2024
Merged

[HLSL] Array by-value assignment #109323

merged 4 commits into from
Oct 1, 2024

Conversation

spall
Copy link
Contributor

@spall spall commented Sep 19, 2024

Make Constant Arrays in HLSL assignable.
Closes #109043

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" HLSL HLSL Language Support labels Sep 19, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 19, 2024

@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: Sarah Spall (spall)

Changes

Make Constant Arrays in HLSL assignable.
Closes #109043


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

6 Files Affected:

  • (modified) clang/include/clang/AST/CanonicalType.h (+1)
  • (modified) clang/lib/AST/ExprClassification.cpp (+2-1)
  • (modified) clang/lib/Sema/SemaOverload.cpp (+8-3)
  • (added) clang/test/AST/HLSL/ArrayAssignable.hlsl (+80)
  • (added) clang/test/CodeGenHLSL/ArrayAssignable.hlsl (+50)
  • (added) clang/test/SemaHLSL/ArrayAssignable_errors.hlsl (+29)
diff --git a/clang/include/clang/AST/CanonicalType.h b/clang/include/clang/AST/CanonicalType.h
index dde08f0394c98d..6102eb01793530 100644
--- a/clang/include/clang/AST/CanonicalType.h
+++ b/clang/include/clang/AST/CanonicalType.h
@@ -299,6 +299,7 @@ class CanProxyBase {
   LLVM_CLANG_CANPROXY_SIMPLE_ACCESSOR(bool, isDependentType)
   LLVM_CLANG_CANPROXY_SIMPLE_ACCESSOR(bool, isOverloadableType)
   LLVM_CLANG_CANPROXY_SIMPLE_ACCESSOR(bool, isArrayType)
+  LLVM_CLANG_CANPROXY_SIMPLE_ACCESSOR(bool, isConstantArrayType)
   LLVM_CLANG_CANPROXY_SIMPLE_ACCESSOR(bool, hasPointerRepresentation)
   LLVM_CLANG_CANPROXY_SIMPLE_ACCESSOR(bool, hasObjCPointerRepresentation)
   LLVM_CLANG_CANPROXY_SIMPLE_ACCESSOR(bool, hasIntegerRepresentation)
diff --git a/clang/lib/AST/ExprClassification.cpp b/clang/lib/AST/ExprClassification.cpp
index 5dde923312698f..9d97633309ada2 100644
--- a/clang/lib/AST/ExprClassification.cpp
+++ b/clang/lib/AST/ExprClassification.cpp
@@ -704,7 +704,8 @@ static Cl::ModifiableType IsModifiable(ASTContext &Ctx, const Expr *E,
     return Cl::CM_ConstAddrSpace;
 
   // Arrays are not modifiable, only their elements are.
-  if (CT->isArrayType())
+  if (CT->isArrayType() &&
+      !(Ctx.getLangOpts().HLSL && CT->isConstantArrayType()))
     return Cl::CM_ArrayType;
   // Incomplete types are not modifiable.
   if (CT->isIncompleteType())
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index d304f322aced64..ce503f9c69b411 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -2232,16 +2232,21 @@ static bool IsStandardConversion(Sema &S, Expr* From, QualType ToType,
     // just strip the qualifiers because they don't matter.
     FromType = FromType.getUnqualifiedType();
   } else if (S.getLangOpts().HLSL && FromType->isConstantArrayType() &&
-             ToType->isArrayParameterType()) {
+             ToType->isConstantArrayType()) {
     // HLSL constant array parameters do not decay, so if the argument is a
     // constant array and the parameter is an ArrayParameterType we have special
     // handling here.
-    FromType = S.Context.getArrayParameterType(FromType);
+    if (ToType->isArrayParameterType()) {
+      FromType = S.Context.getArrayParameterType(FromType);
+      SCS.First = ICK_HLSL_Array_RValue;
+    } else {
+      SCS.First = ICK_Identity;
+    }
+
     if (S.Context.getCanonicalType(FromType) !=
         S.Context.getCanonicalType(ToType))
       return false;
 
-    SCS.First = ICK_HLSL_Array_RValue;
     SCS.setAllToTypes(ToType);
     return true;
   } else if (FromType->isArrayType()) {
diff --git a/clang/test/AST/HLSL/ArrayAssignable.hlsl b/clang/test/AST/HLSL/ArrayAssignable.hlsl
new file mode 100644
index 00000000000000..52c9918aa85334
--- /dev/null
+++ b/clang/test/AST/HLSL/ArrayAssignable.hlsl
@@ -0,0 +1,80 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -ast-dump %s | FileCheck %s
+
+// CHECK-LABEL: arr_assign1
+// CHECK: CompoundStmt 0x{{[0-9a-f]+}} {{.*}}
+// CHECK: DeclStmt 0x{{[0-9a-f]+}} {{.*}}
+// CHECK: VarDecl [[A:0x[0-9a-f]+]] {{.*}} col:7 used Arr 'int[2]' cinit
+// CHECK: InitListExpr 0x{{[0-9a-f]+}} {{.*}} 'int[2]'
+// CHECK: IntegerLiteral 0x{{[0-9a-f]+}} {{.*}} 'int' 0
+// CHECK: IntegerLiteral 0x{{[0-9a-f]+}} {{.*}} 'int' 1
+// CHECK: DeclStmt 0x{{[0-9a-f]+}} {{.*}}
+// CHECK: VarDecl [[B:0x[0-9a-f]+]] {{.*}} col:7 used Arr2 'int[2]' cinit
+// CHECK: InitListExpr 0x{{[0-9a-f]+}} {{.*}} 'int[2]'
+// CHECK: IntegerLiteral 0x{{[0-9a-f]+}} {{.*}} 'int' 0
+// CHECK: IntegerLiteral 0x{{[0-9a-f]+}} {{.*}} 'int' 0
+// CHECK: BinaryOperator 0x{{[0-9a-f]+}} {{.*}} 'int[2]' lvalue '='
+// CHECK: DeclRefExpr 0x{{[0-9a-f]+}} {{.*}} 'int[2]' lvalue Var [[A]] 'Arr' 'int[2]'
+// CHECK: DeclRefExpr 0x{{[0-9a-f]+}} {{.*}} 'int[2]' lvalue Var [[B]] 'Arr2' 'int[2]'
+void arr_assign1() {
+  int Arr[2] = {0, 1};
+  int Arr2[2] = {0, 0};
+  Arr = Arr2;
+}
+
+// CHECK-LABEL: arr_assign2
+// CHECK: CompoundStmt 0x{{[0-9a-f]+}} {{.*}}
+// CHECK: DeclStmt 0x{{[0-9a-f]+}} {{.*}}
+// CHECK: VarDecl [[A:0x[0-9a-f]+]] {{.*}} col:7 used Arr 'int[2]' cinit
+// CHECK: InitListExpr 0x{{[0-9a-f]+}} {{.*}} 'int[2]'
+// CHECK: IntegerLiteral 0x{{[0-9a-f]+}} {{.*}} 'int' 0
+// CHECK: IntegerLiteral 0x{{[0-9a-f]+}} {{.*}} 'int' 1
+// CHECK: DeclStmt 0x{{[0-9a-f]+}} {{.*}}
+// CHECK: VarDecl [[B:0x[0-9a-f]+]] {{.*}} col:7 used Arr2 'int[2]' cinit
+// CHECK: InitListExpr 0x{{[0-9a-f]+}} {{.*}} 'int[2]'
+// CHECK: IntegerLiteral 0x{{[0-9a-f]+}} {{.*}} 'int' 0
+// CHECK: IntegerLiteral 0x{{[0-9a-f]+}} {{.*}} 'int' 0
+// CHECK: DeclStmt 0x{{[0-9a-f]+}} {{.*}}
+// CHECK: VarDecl [[C:0x[0-9a-f]+]] {{.*}} col:7 used Arr3 'int[2]' cinit
+// CHECK: InitListExpr 0x{{[0-9a-f]+}} {{.*}} 'int[2]'
+// CHECK: IntegerLiteral 0x{{[0-9a-f]+}} {{.*}} 'int' 2
+// CHECK: IntegerLiteral 0x{{[0-9a-f]+}} {{.*}} 'int' 2
+// CHECK: BinaryOperator 0x{{[0-9a-f]+}} {{.*}} 'int[2]' lvalue '='
+// CHECK: DeclRefExpr 0x{{[0-9a-f]+}} {{.*}} 'int[2]' lvalue Var [[A]] 'Arr' 'int[2]'
+// CHECK: BinaryOperator 0x{{[0-9a-f]+}} {{.*}} 'int[2]' lvalue '='
+// CHECK: DeclRefExpr 0x{{[0-9a-f]+}} {{.*}} 'int[2]' lvalue Var [[B]] 'Arr2' 'int[2]'
+// CHECK: DeclRefExpr 0x{{[0-9a-f]+}} {{.*}} 'int[2]' lvalue Var [[C]] 'Arr3' 'int[2]'
+void arr_assign2() {
+  int Arr[2] = {0, 1};
+  int Arr2[2] = {0, 0};
+  int Arr3[2] = {2, 2};
+  Arr = Arr2 = Arr3;
+}
+
+// CHECK-LABEL: arr_assign3
+// CHECK: CompoundStmt 0x{{[0-9a-f]+}} {{.*}}
+// CHECK: DeclStmt 0x{{[0-9a-f]+}} {{.*}}
+// CHECK: VarDecl [[A:0x[0-9a-f]+]] {{.*}} col:7 used Arr 'int[2][2]' cinit
+// CHECK: InitListExpr 0x{{[0-9a-f]+}} {{.*}} 'int[2][2]'
+// CHECK: InitListExpr 0x{{[0-9a-f]+}} {{.*}} 'int[2]'
+// CHECK: IntegerLiteral 0x{{[0-9a-f]+}} {{.*}} 'int' 0
+// CHECK: IntegerLiteral 0x{{[0-9a-f]+}} {{.*}} 'int' 1
+// CHECK: InitListExpr 0x{{[0-9a-f]+}} {{.*}} 'int[2]'
+// CHECK: IntegerLiteral 0x{{[0-9a-f]+}} {{.*}} 'int' 2
+// CHECK: IntegerLiteral 0x{{[0-9a-f]+}} {{.*}} 'int' 3
+// CHECK: DeclStmt 0x{{[0-9a-f]+}} {{.*}}
+// CHECK: VarDecl [[B:0x[0-9a-f]+]] {{.*}} col:7 used Arr2 'int[2][2]' cinit
+// CHECK: InitListExpr 0x{{[0-9a-f]+}} {{.*}} 'int[2][2]'
+// CHECK: InitListExpr 0x{{[0-9a-f]+}} {{.*}} 'int[2]'
+// CHECK: IntegerLiteral 0x{{[0-9a-f]+}} {{.*}} 'int' 0
+// CHECK: IntegerLiteral 0x{{[0-9a-f]+}} {{.*}} 'int' 0
+// CHECK: InitListExpr 0x{{[0-9a-f]+}} {{.*}} 'int[2]'
+// CHECK: IntegerLiteral 0x{{[0-9a-f]+}} {{.*}} 'int' 1
+// CHECK: IntegerLiteral 0x{{[0-9a-f]+}} {{.*}} 'int' 1
+// CHECK: BinaryOperator 0x{{[0-9a-f]+}} {{.*}} 'int[2][2]' lvalue '='
+// CHECK: DeclRefExpr 0x{{[0-9a-f]+}} {{.*}} 'int[2][2]' lvalue Var [[A]] 'Arr' 'int[2][2]'
+// CHECK: DeclRefExpr 0x{{[0-9a-f]+}} {{.*}} 'int[2][2]' lvalue Var [[B]] 'Arr2' 'int[2][2]'
+void arr_assign3() {
+  int Arr[2][2] = {{0, 1}, {2, 3}};
+  int Arr2[2][2] = {{0, 0}, {1, 1}};
+  Arr = Arr2;
+}
\ No newline at end of file
diff --git a/clang/test/CodeGenHLSL/ArrayAssignable.hlsl b/clang/test/CodeGenHLSL/ArrayAssignable.hlsl
new file mode 100644
index 00000000000000..769154df2bfb43
--- /dev/null
+++ b/clang/test/CodeGenHLSL/ArrayAssignable.hlsl
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s --enable-var-scope
+
+// CHECK-LABEL: define void {{.*}}arr_assign1
+// CHECK: [[Arr:%.*]] = alloca [2 x i32], align 4
+// CHECK: [[Arr2:%.*]] = alloca [2 x i32], align 4
+// CHECK: [[Tmp:%.*]] = alloca [2 x i32], align 4
+// CHECK: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Arr]], ptr align 4 {{@.*}}, i32 8, i1 false)
+// CHECK: call void @llvm.memset.p0.i32(ptr align 4 [[Arr2]], i8 0, i32 8, i1 false)
+// CHECK: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Arr]], ptr align 4 [[Arr2]], i32 8, i1 false)
+// CHECK: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Tmp]], ptr align 4 [[Arr]], i32 8, i1 false)
+// CHECK: ret void
+void arr_assign1() {
+  int Arr[2] = {0, 1};
+  int Arr2[2] = {0, 0};
+  Arr = Arr2;
+}
+
+// CHECK-LABEL: define void {{.*}}arr_assign2
+// CHECK: [[Arr:%.*]] = alloca [2 x i32], align 4
+// CHECK: [[Arr2:%.*]] = alloca [2 x i32], align 4
+// CHECK: [[Arr3:%.*]] = alloca [2 x i32], align 4
+// CHECK: [[Tmp:%.*]] = alloca [2 x i32], align 4
+// CHECK: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Arr]], ptr align 4 {{@.*}}, i32 8, i1 false)
+// CHECK: call void @llvm.memset.p0.i32(ptr align 4 [[Arr2]], i8 0, i32 8, i1 false)
+// CHECK: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Arr3]], ptr align 4 {{@.*}}, i32 8, i1 false)
+// CHECK: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Arr2]], ptr align 4 [[Arr3]], i32 8, i1 false)
+// CHECK: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Arr]], ptr align 4 [[Arr2]], i32 8, i1 false)
+// CHECK: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Tmp]], ptr align 4 [[Arr]], i32 8, i1 false)
+// CHECK: ret void
+void arr_assign2() {
+  int Arr[2] = {0, 1};
+  int Arr2[2] = {0, 0};
+  int Arr3[2] = {3, 4};
+  Arr = Arr2 = Arr3;
+}
+
+// CHECK-LABEL: define void {{.*}}arr_assign3
+// CHECK: [[Arr3:%.*]] = alloca [2 x [2 x i32]], align 4
+// CHECK: [[Arr4:%.*]] = alloca [2 x [2 x i32]], align 4
+// CHECK: [[Tmp:%.*]] = alloca [2 x [2 x i32]], align 4
+// CHECK: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Arr3]], ptr align 4 {{@.*}}, i32 16, i1 false)
+// CHECK: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Arr4]], ptr align 4 {{@.*}}, i32 16, i1 false)
+// CHECK: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Arr3]], ptr align 4 [[Arr4]], i32 16, i1 false)
+// CHECK: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Tmp]], ptr align 4 [[Arr3]], i32 16, i1 false)
+// CHECK: ret void
+void arr_assign3() {
+  int Arr2[2][2] = {{0, 0}, {1, 1}};
+  int Arr3[2][2] = {{1, 1}, {0, 0}};
+  Arr2 = Arr3;
+}
diff --git a/clang/test/SemaHLSL/ArrayAssignable_errors.hlsl b/clang/test/SemaHLSL/ArrayAssignable_errors.hlsl
new file mode 100644
index 00000000000000..1925032a93d488
--- /dev/null
+++ b/clang/test/SemaHLSL/ArrayAssignable_errors.hlsl
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -verify
+
+void test_wrong_size1() {
+  int Arr[2] = {0, 1};
+  int Arr2[3] = {1, 2, 0};
+  Arr = Arr2;
+  // expected-error@-1 {{assigning to 'int[2]' from incompatible type 'int[3]'}}
+}
+
+void test_wrong_size2() {
+  int Arr[2] = {0, 1};
+  int Arr2[3] = {1, 2, 0};
+  Arr2 = Arr;
+  // expected-error@-1 {{assigning to 'int[3]' from incompatible type 'int[2]'}}
+}
+
+void test_wrong_size3() {
+  int Arr[2][2] = {{0, 1}, {2, 3}};
+  int Arr2[2] = {4, 5};
+  Arr = Arr2;
+  // expected-error@-1 {{assigning to 'int[2][2]' from incompatible type 'int[2]'}}
+}
+
+void test_wrong_size4() {
+  int Arr[2][2] = {{0, 1}, {2, 3}};
+  int Arr2[2] = {4, 5};
+  Arr2 = Arr;
+  // expected-error@-1 {{assigning to 'int[2]' from incompatible type 'int[2][2]'}}
+}

bogner
bogner previously approved these changes Sep 23, 2024
Copy link
Collaborator

@llvm-beanz llvm-beanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We either need to fix code generation so that we correctly handle modifications of the binary operator's lvalue, or we need to represent the binary operator in the AST as returning an rvalue.

This code should either work following C++ rules, or fail to compile following C rules:

export int fn() {
  int Arr[2] = {0, 1};
  int Arr2[2] = {1, 2};
  (Arr = Arr2)[0] = 6;
  return Arr[0] + Arr[1];
}

@bogner bogner dismissed their stale review September 23, 2024 22:12

We seem to be getting a mix of C and C++ semantics here

Copy link
Collaborator

@llvm-beanz llvm-beanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few notes on the test cases, but otherwise I think this is looking good.

@spall spall merged commit d8df118 into llvm:main Oct 1, 2024
8 checks passed
VitaNuo pushed a commit to VitaNuo/llvm-project that referenced this pull request Oct 2, 2024
Make Constant Arrays in HLSL assignable. 
Closes llvm#109043
VitaNuo pushed a commit to VitaNuo/llvm-project that referenced this pull request Oct 2, 2024
Make Constant Arrays in HLSL assignable. 
Closes llvm#109043
Sterling-Augustine pushed a commit to Sterling-Augustine/llvm-project that referenced this pull request Oct 3, 2024
Make Constant Arrays in HLSL assignable. 
Closes llvm#109043
xgupta pushed a commit to xgupta/llvm-project that referenced this pull request Oct 4, 2024
Make Constant Arrays in HLSL assignable. 
Closes llvm#109043
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

[HLSL] Array by-value assignment
4 participants