[CIR] Upstream CIR codegen for vec_ext x86 builtins

[Thibault-Monnier]

This PR upstreams the codegen for the x86 vec_ext builtins from the incubator. It is part of #167752.

[llvmbot]

@llvm/pr-subscribers-clangir

@llvm/pr-subscribers-clang

Author: Thibault Monnier (Thibault-Monnier)

Changes

This PR upstreams the codegen for the x86 vec_ext builtins from the incubator. It is part of #167752.

---

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

5 Files Affected:

• (modified) clang/include/clang/CIR/Dialect/IR/CIROps.td (+6)
• (modified) clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp (+16)
• (modified) clang/lib/CIR/CodeGen/CIRGenBuiltinX86.cpp (+34-2)
• (modified) clang/lib/CIR/CodeGen/CIRGenFunction.h (+3)
• (added) clang/test/CIR/CodeGen/X86/sse2-builtins.c (+27)

diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index 16258513239d9..9646e55ab9ea8 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -413,6 +413,12 @@ def CIR_ConstantOp : CIR_Op<"const", [
 
     template <typename T>
     T getValueAttr() { return mlir::dyn_cast<T>(getValue()); }
+
+    llvm::APInt getIntValue() {
+      if (const auto intAttr = getValueAttr<cir::IntAttr>())
+        return intAttr.getValue();
+      llvm_unreachable("Expected an IntAttr in ConstantOp");
+    }
   }];
 
   let hasFolder = 1;
diff --git a/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp b/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp
index 4e6a5ee7ee210..b54256715be96 100644
--- a/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp
@@ -625,6 +625,22 @@ CIRGenFunction::emitTargetBuiltinExpr(unsigned builtinID, const CallExpr *e,
                                    getTarget().getTriple().getArch());
 }
 
+mlir::Value CIRGenFunction::emitScalarOrConstFoldImmArg(
+    const unsigned iceArguments, const unsigned idx, const Expr *argExpr) {
+  mlir::Value arg = {};
+  if ((iceArguments & (1 << idx)) == 0) {
+    arg = emitScalarExpr(argExpr);
+  } else {
+    // If this is required to be a constant, constant fold it so that we
+    // know that the generated intrinsic gets a ConstantInt.
+    const std::optional<llvm::APSInt> result =
+        argExpr->getIntegerConstantExpr(getContext());
+    assert(result && "Expected argument to be a constant");
+    arg = builder.getConstInt(getLoc(argExpr->getSourceRange()), *result);
+  }
+  return arg;
+}
+
 /// Given a builtin id for a function like "__builtin_fabsf", return a Function*
 /// for "fabsf".
 cir::FuncOp CIRGenModule::getBuiltinLibFunction(const FunctionDecl *fd,
diff --git a/clang/lib/CIR/CodeGen/CIRGenBuiltinX86.cpp b/clang/lib/CIR/CodeGen/CIRGenBuiltinX86.cpp
index 0198a9d4eb192..59f709b8270dd 100644
--- a/clang/lib/CIR/CodeGen/CIRGenBuiltinX86.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenBuiltinX86.cpp
@@ -16,7 +16,6 @@
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/TargetBuiltins.h"
 #include "clang/CIR/MissingFeatures.h"
-#include "llvm/IR/IntrinsicsX86.h"
 
 using namespace clang;
 using namespace clang::CIRGen;
@@ -43,6 +42,18 @@ mlir::Value CIRGenFunction::emitX86BuiltinExpr(unsigned builtinID,
   // Find out if any arguments are required to be integer constant expressions.
   assert(!cir::MissingFeatures::handleBuiltinICEArguments());
 
+  llvm::SmallVector<mlir::Value> ops;
+
+  // Find out if any arguments are required to be integer constant expressions.
+  unsigned iceArguments = 0;
+  ASTContext::GetBuiltinTypeError error;
+  getContext().GetBuiltinType(builtinID, error, &iceArguments);
+  assert(error == ASTContext::GE_None && "Should not codegen an error");
+
+  for (auto [idx, arg] : llvm::enumerate(e->arguments())) {
+    ops.push_back(emitScalarOrConstFoldImmArg(iceArguments, idx, arg));
+  }
+
   switch (builtinID) {
   default:
     return {};
@@ -63,6 +74,10 @@ mlir::Value CIRGenFunction::emitX86BuiltinExpr(unsigned builtinID,
   case X86::BI__builtin_ia32_undef128:
   case X86::BI__builtin_ia32_undef256:
   case X86::BI__builtin_ia32_undef512:
+    cgm.errorNYI(e->getSourceRange(),
+                 std::string("unimplemented X86 builtin call: ") +
+                     getContext().BuiltinInfo.getName(builtinID));
+    return {};
   case X86::BI__builtin_ia32_vec_ext_v4hi:
   case X86::BI__builtin_ia32_vec_ext_v16qi:
   case X86::BI__builtin_ia32_vec_ext_v8hi:
@@ -72,7 +87,24 @@ mlir::Value CIRGenFunction::emitX86BuiltinExpr(unsigned builtinID,
   case X86::BI__builtin_ia32_vec_ext_v32qi:
   case X86::BI__builtin_ia32_vec_ext_v16hi:
   case X86::BI__builtin_ia32_vec_ext_v8si:
-  case X86::BI__builtin_ia32_vec_ext_v4di:
+  case X86::BI__builtin_ia32_vec_ext_v4di: {
+    unsigned NumElts = cast<cir::VectorType>(ops[0].getType()).getSize();
+
+    uint64_t index =
+        ops[1].getDefiningOp<cir::ConstantOp>().getIntValue().getZExtValue();
+
+    index &= NumElts - 1;
+
+    auto indexAttr = cir::IntAttr::get(
+        cir::IntType::get(&getMLIRContext(), 64, false), index);
+    auto indexVal =
+        cir::ConstantOp::create(builder, getLoc(e->getExprLoc()), indexAttr);
+
+    // These builtins exist so we can ensure the index is an ICE and in range.
+    // Otherwise we could just do this in the header file.
+    return cir::VecExtractOp::create(builder, getLoc(e->getExprLoc()), ops[0],
+                                     indexVal);
+  }
   case X86::BI__builtin_ia32_vec_set_v4hi:
   case X86::BI__builtin_ia32_vec_set_v16qi:
   case X86::BI__builtin_ia32_vec_set_v8hi:
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index f879e580989f7..c2ef98d2b25d6 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -1699,6 +1699,9 @@ class CIRGenFunction : public CIRGenTypeCache {
   void emitScalarInit(const clang::Expr *init, mlir::Location loc,
                       LValue lvalue, bool capturedByInit = false);
 
+  mlir::Value emitScalarOrConstFoldImmArg(unsigned iceArguments, unsigned idx,
+                                          const Expr *argExpr);
+
   void emitStaticVarDecl(const VarDecl &d, cir::GlobalLinkageKind linkage);
 
   void emitStoreOfComplex(mlir::Location loc, mlir::Value v, LValue dest,
diff --git a/clang/test/CIR/CodeGen/X86/sse2-builtins.c b/clang/test/CIR/CodeGen/X86/sse2-builtins.c
new file mode 100644
index 0000000000000..3af8bfc57f01c
--- /dev/null
+++ b/clang/test/CIR/CodeGen/X86/sse2-builtins.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -x c -flax-vector-conversions=none -ffreestanding %s -triple=x86_64-unknown-linux -target-feature +sse2 -fclangir -emit-cir -o %t.cir -Wall -Werror
+// RUN: FileCheck --check-prefixes=CIR-CHECK --input-file=%t.cir %s
+// RUN: %clang_cc1 -x c -flax-vector-conversions=none -ffreestanding %s -triple=x86_64-unknown-linux -target-feature +sse2 -fno-signed-char -fclangir -emit-cir -o %t.cir -Wall -Werror
+// RUN: FileCheck --check-prefixes=CIR-CHECK --input-file=%t.cir %s
+
+// RUN: %clang_cc1 -x c++ -flax-vector-conversions=none -ffreestanding %s -triple=x86_64-unknown-linux -target-feature +sse2 -fclangir -emit-llvm -o %t.ll -Wall -Werror
+// RUN: FileCheck --check-prefixes=LLVM-CHECK --input-file=%t.ll %s
+// RUN: %clang_cc1 -x c++ -flax-vector-conversions=none -ffreestanding %s -triple=x86_64-unknown-linux -target-feature +sse2 -fno-signed-char -fclangir -emit-llvm -o %t.ll -Wall -Werror
+// RUN: FileCheck --check-prefixes=LLVM-CHECK --input-file=%t.ll %s
+
+// This test mimics clang/test/CodeGen/X86/sse2-builtins.c, which eventually
+// CIR shall be able to support fully.
+
+#include <immintrin.h>
+
+// Lowering to pextrw requires optimization.
+int test_mm_extract_epi16(__m128i A) {
+
+  // CIR-CHECK-LABEL: test_mm_extract_epi16
+  // CIR-CHECK %{{.*}} = cir.vec.extract %{{.*}}[%{{.*}} : {{!u32i|!u64i}}] : !cir.vector<!s16i x 8>
+  // CIR-CHECK %{{.*}} = cir.cast integral %{{.*}} : !u16i -> !s32i
+
+  // LLVM-CHECK-LABEL: test_mm_extract_epi16
+  // LLVM-CHECK: extractelement <8 x i16> %{{.*}}, {{i32|i64}} 1
+  // LLVM-CHECK: zext i16 %{{.*}} to i32
+  return _mm_extract_epi16(A, 1);
+}

[Thibault-Monnier]

@andykaylor I found it easier to work from a clean PR. I have closed the previous one.

[andykaylor]

This looks good. It's small enough to effectively review, and it does a single thing. I just have a few requests for changes.

[Thibault-Monnier]

@andykaylor I've applied your suggestions, and by the same occasion, trivially refactored getSInt64 (because I implemented getUInt64).

By the way, should I keep using an amend commit after the review to keep the commit tree clean?

[andykaylor]

lgtm

[andykaylor]

The CI build failure should be fixed by #167969

[andykaylor]

By the way, should I keep using an amend commit after the review to keep the commit tree clean?

@Thibault-Monnier Sorry, I missed this question yesterday. No, it's much better to just add additional commits to your review branch and avoid rebasing or other merges while the review is ongoing. That makes it easier to see what has changed since the code was last reviewed. That's not a big deal for small PRs like this, but it becomes very important on larger PRs, especially if the changes are substantial. The individual commits get squashed together when the PR is merged. If there are conflicts, you can rebase and force push to your review branch after the PR is approved.

[Thibault-Monnier]

@andykaylor I'm done. Please merge on my behalf if you are satisfied with this PR.