[CIR] Upstream Vector support in elementTypeIfVector

[AmrDeveloper]

Upstream vector support in the element type if vector as a required change for upstreaming other Vec Ops

Issue #136487

[llvmbot]

@llvm/pr-subscribers-clang

Author: Amr Hesham (AmrDeveloper)

Changes

Upstream vector support in the element type if vector as a required change for upstreaming other Vec Ops

Issue #136487

---

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

1 Files Affected:

• (modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp (+5-3)

diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index 3c85bb4b6b41d..c3f8ad2521d1d 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -54,10 +54,12 @@ namespace direct {
 namespace {
 /// If the given type is a vector type, return the vector's element type.
 /// Otherwise return the given type unchanged.
-// TODO(cir): Return the vector element type once we have support for vectors
-// instead of the identity type.
 mlir::Type elementTypeIfVector(mlir::Type type) {
-  assert(!cir::MissingFeatures::vectorType());
+  if (auto vecType = mlir::dyn_cast<cir::VectorType>(type))
+    return vecType.getElementType();
+
+  if (auto vecType = mlir::dyn_cast<mlir::VectorType>(type))
+    return vecType.getElementType();
   return type;
 }
 } // namespace

[llvmbot]

@llvm/pr-subscribers-clangir

Author: Amr Hesham (AmrDeveloper)

Changes

Upstream vector support in the element type if vector as a required change for upstreaming other Vec Ops

Issue #136487

---

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

1 Files Affected:

• (modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp (+5-3)

diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index 3c85bb4b6b41d..c3f8ad2521d1d 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -54,10 +54,12 @@ namespace direct {
 namespace {
 /// If the given type is a vector type, return the vector's element type.
 /// Otherwise return the given type unchanged.
-// TODO(cir): Return the vector element type once we have support for vectors
-// instead of the identity type.
 mlir::Type elementTypeIfVector(mlir::Type type) {
-  assert(!cir::MissingFeatures::vectorType());
+  if (auto vecType = mlir::dyn_cast<cir::VectorType>(type))
+    return vecType.getElementType();
+
+  if (auto vecType = mlir::dyn_cast<mlir::VectorType>(type))
+    return vecType.getElementType();
   return type;
 }
 } // namespace

[andykaylor]

In case it wasn't clear from my comment on the binop lowering PR, I was suggesting that the unary op and bin op PRs could be combined, but I said that before I saw this one. Since you've already done this, let's just keep them separate.

[andykaylor]

I've looked at this twice now, and both times I thought these two if statements were identical. It took a third look to see that one is checking for cir::VectorType and the other is checking for mlir::VectorType. Maybe add a comment drawing attention to this fact?

[AmrDeveloper]

Yes, it was confusing for me too, especially when I got the exception, it was weird why it didn't return the element type :D

[andykaylor]

Can you implement this with a TypeSwitch?

return llvm::TypeSwitch<mlir::Type, mlir::Type>(type)
           .Case<cir::VectorType, mlir::VectorType>([](auto p) { return p->getElementType(); }
           .Default([](mlir::Type p) { return p; });

[AmrDeveloper]

Done