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

[Clang][AArch64][SVE] Allow write to SVE vector elements using the subscript operator #91965

Merged
merged 2 commits into from
May 22, 2024

Conversation

momchil-velikov
Copy link
Collaborator

The patch at https://reviews.llvm.org/D122732 introduced using the array subscript operator for SVE vectors, however it also causes an ICE when the subscripting expression is used as an lvalue.

This patches fixes the error. Lvalue subscripting expressions are emitted as LLVM IR insertelement.

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

llvmbot commented May 13, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: Momchil Velikov (momchil-velikov)

Changes

The patch at https://reviews.llvm.org/D122732 introduced using the array subscript operator for SVE vectors, however it also causes an ICE when the subscripting expression is used as an lvalue.

This patches fixes the error. Lvalue subscripting expressions are emitted as LLVM IR insertelement.


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

3 Files Affected:

  • (modified) clang/lib/CodeGen/CGExpr.cpp (+4-2)
  • (modified) clang/lib/Sema/SemaExpr.cpp (+3-1)
  • (modified) clang/test/CodeGen/aarch64-sve-vector-subscript-ops.c (+10)
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index d96c7bb1e5682..37b8b723937b7 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -4180,8 +4180,10 @@ LValue CodeGenFunction::EmitArraySubscriptExpr(const ArraySubscriptExpr *E,
 
   // If the base is a vector type, then we are forming a vector element lvalue
   // with this subscript.
-  if (E->getBase()->getType()->isVectorType() &&
-      !isa<ExtVectorElementExpr>(E->getBase())) {
+  if (QualType BaseTy = E->getBase()->getType();
+      (BaseTy->isVectorType() && !isa<ExtVectorElementExpr>(E->getBase())) ||
+      (BaseTy->isBuiltinType() &&
+       BaseTy->getAs<BuiltinType>()->isSveVLSBuiltinType())) {
     // Emit the vector as an lvalue to get its address.
     LValue LHS = EmitLValue(E->getBase());
     auto *Idx = EmitIdxAfterBase(/*Promote*/false);
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index bb4b116fd73ca..fd16be30bd848 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5383,7 +5383,9 @@ Sema::CreateBuiltinArraySubscriptExpr(Expr *Base, SourceLocation LLoc,
   }
 
   // Perform default conversions.
-  if (!LHSExp->getType()->getAs<VectorType>()) {
+  if (!LHSExp->getType()->getAs<VectorType>() &&
+      !(LHSExp->getType()->isBuiltinType() &&
+        LHSExp->getType()->getAs<BuiltinType>()->isSveVLSBuiltinType())) {
     ExprResult Result = DefaultFunctionArrayLvalueConversion(LHSExp);
     if (Result.isInvalid())
       return ExprError();
diff --git a/clang/test/CodeGen/aarch64-sve-vector-subscript-ops.c b/clang/test/CodeGen/aarch64-sve-vector-subscript-ops.c
index fb60c6d100ce6..634423765c4c3 100644
--- a/clang/test/CodeGen/aarch64-sve-vector-subscript-ops.c
+++ b/clang/test/CodeGen/aarch64-sve-vector-subscript-ops.c
@@ -88,3 +88,13 @@ float subscript_float32(svfloat32_t a, size_t b) {
 double subscript_float64(svfloat64_t a, size_t b) {
   return a[b];
 }
+
+// CHECK-LABEL: @subscript_write_float32(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[VECINS:%.*]] = insertelement <vscale x 4 x float> [[A:%.*]], float 1.000000e+00, i64 [[B:%.*]]
+// CHECK-NEXT:    ret <vscale x 4 x float> [[VECINS]]
+//
+svfloat32_t subscript_write_float32(svfloat32_t a, size_t b) {
+  a[b] = 1.0f;
+  return a;
+}

Copy link
Member

@DavidTruby DavidTruby left a comment

Choose a reason for hiding this comment

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

LGTM

@momchil-velikov momchil-velikov force-pushed the sve-array-subscript-assign branch 2 times, most recently from fec051f to 3f1e962 Compare May 20, 2024 15:59
…bscript operator

The patch at https://reviews.llvm.org/D122732 introduced using
the array subscript operator for SVE vectors, however it also
causes an ICE when the subscripting expression is used
as an lvalue.

This patches fixes the error. Lvalue subscripting expressions are
emitted as LLVM IR `insertvector`.

Change-Id: I46d0333d8ed8508cd9cd23e02dd1c2d48fb74cd2
Change-Id: I81e1fd4f23eb65a96e71015de7a4562fcbc53c0f
@momchil-velikov
Copy link
Collaborator Author

Rebased.

Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

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

LGTM

@momchil-velikov momchil-velikov merged commit 91d415b into llvm:main May 22, 2024
3 of 4 checks passed
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants