[CIR] Upstream CIR codegen for undef x86 builtins

[Thibault-Monnier]

... It is part of #167752.

[llvmbot]

@llvm/pr-subscribers-clangir

@llvm/pr-subscribers-clang

Author: Thibault Monnier (Thibault-Monnier)

Changes

... It is part of #167752.

---

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

2 Files Affected:

• (modified) clang/lib/CIR/CodeGen/CIRGenBuiltinX86.cpp (+11-1)
• (added) clang/test/CIR/CodeGen/X86/sse2-builtins.c (+39)

diff --git a/clang/lib/CIR/CodeGen/CIRGenBuiltinX86.cpp b/clang/lib/CIR/CodeGen/CIRGenBuiltinX86.cpp
index 0198a9d4eb192..91417c2d192e7 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;
@@ -60,9 +59,20 @@ mlir::Value CIRGenFunction::emitX86BuiltinExpr(unsigned builtinID,
   case X86::BI__builtin_ia32_tzcnt_u16:
   case X86::BI__builtin_ia32_tzcnt_u32:
   case X86::BI__builtin_ia32_tzcnt_u64:
+    cgm.errorNYI(e->getSourceRange(),
+                 std::string("unimplemented X86 builtin call: ") +
+                     getContext().BuiltinInfo.getName(builtinID));
+    return {};
   case X86::BI__builtin_ia32_undef128:
   case X86::BI__builtin_ia32_undef256:
   case X86::BI__builtin_ia32_undef512:
+    // The x86 definition of "undef" is not the same as the LLVM definition
+    // (PR32176). We leave optimizing away an unnecessary zero constant to the
+    // IR optimizer and backend.
+    // TODO: If we had a "freeze" IR instruction to generate a fixed undef
+    //  value, we should use that here instead of a zero.
+    return builder.getNullValue(convertType(e->getType()),
+                                getLoc(e->getExprLoc()));
   case X86::BI__builtin_ia32_vec_ext_v4hi:
   case X86::BI__builtin_ia32_vec_ext_v16qi:
   case X86::BI__builtin_ia32_vec_ext_v8hi:
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..eea1a5f6149b5
--- /dev/null
+++ b/clang/test/CIR/CodeGen/X86/sse2-builtins.c
@@ -0,0 +1,39 @@
+// 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,CIR-X64 --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,CIR-X64 --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,LLVM-X64 --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,LLVM-X64 --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>
+
+__m128d test_mm_undefined_pd(void) {
+  // CIR-X64-LABEL: _mm_undefined_pd
+  // CIR-X64: %{{.*}} = cir.const #cir.zero : !cir.vector<2 x !cir.double>
+  // CIR-X64: cir.return %{{.*}} : !cir.vector<2 x !cir.double>
+
+  // LLVM-X64-LABEL: test_mm_undefined_pd
+  // LLVM-X64: store <2 x double> zeroinitializer, ptr %[[A:.*]], align 16
+  // LLVM-X64: %{{.*}} = load <2 x double>, ptr %[[A]], align 16
+  // LLVM-X64: ret <2 x double> %{{.*}}
+  return _mm_undefined_pd();
+}
+
+__m128i test_mm_undefined_si128(void) {
+  // CIR-LABEL: _mm_undefined_si128
+  // CIR-CHECK: %[[A:.*]] = cir.const #cir.zero : !cir.vector<2 x !cir.double>
+  // CIR-CHECK: %{{.*}} = cir.cast bitcast %[[A]] : !cir.vector<2 x !cir.double> -> !cir.vector<2 x !s64i>
+  // CIR-CHECK: cir.return %{{.*}} : !cir.vector<2 x !s64i>
+
+  // LLVM-CHECK-LABEL: test_mm_undefined_si128
+  // LLVM-CHECK: store <2 x i64> zeroinitializer, ptr %[[A:.*]], align 16
+  // LLVM-CHECK: %{{.*}} = load <2 x i64>, ptr %[[A]], align 16
+  // LLVM-CHECK: ret <2 x i64> %{{.*}}
+  return _mm_undefined_si128();
+}

[Thibault-Monnier]

@andykaylor

[andykaylor]

lgtm

[Thibault-Monnier]

The tests are failing, but I do not understand why...

[andykaylor]

The tests are failing, but I do not understand why...

CIR doesn't perform inlining. When we emit LLVM, there will be some inlining performed (unless we explicitly prevent it by disabling LLVM passes). The X86 intrinsics, like _mm_undefined_pd are typically defined in the immintrin.h and sub-included headers as functions that always get inlined.

static __inline__ __m128d __DEFAULT_FN_ATTRS _mm_undefined_pd(void) {
  return (__m128d)__builtin_ia32_undef128();
}

The incubator sse2-builtins.c test, the _mm_undefined_pd test looks like this:

__m128d test_mm_undefined_pd(void) {
  // CIR-X64-LABEL: _mm_undefined_pd
  // CIR-X64: %{{.*}} = cir.const #cir.zero : !cir.vector<!cir.double x 2>
  // CIR-X64: cir.return %{{.*}} : !cir.vector<!cir.double x 2>

  // LLVM-X64-LABEL: test_mm_undefined_pd
  // LLVM-X64: store <2 x double> zeroinitializer, ptr %[[A:.*]], align 16
  // LLVM-X64: %{{.*}} = load <2 x double>, ptr %[[A]], align 16
  // LLVM-X64: ret <2 x double> %{{.*}}
  return _mm_undefined_pd();
}

Notice that in the CIR case, it is checking for _mm_undefined_pd rather than test_mm_undefined_pd. That's because it's checking the pre-inlined definition. I'd kind of like to see both checked, like this:

// CIR-LABEL: cir.func ((.*}} _mm_undefined_pd
// CIR:         %{{.*}} = cir.const #cir.zero : !cir.vector<2 x !cir.double>
// CIR:         cir.return %{{.*}} : !cir.vector<2 x !cir.double>

// CIR-LABEL: cir.func ((.*}} test_mm_undefined_pd
// CIR:         call @_mm_undefined_pd

[Thibault-Monnier]

@andykaylor Thanks for the detailed explanation. Is it OK now?

[andykaylor]

Looks good!

[Thibault-Monnier]

Sorry, I realised I forgot to include some tests from the incubator. I'll take care of that when I can.

[github-actions]

🐧 Linux x64 Test Results

• 112077 tests passed
• 4077 tests skipped

[andykaylor]

@Thibault-Monnier Is this ready to merge after rebasing?

[Thibault-Monnier]

I am in the process of adding the tests I forgot to upstream. This is also the case for the mxcsr builtins PR.

[Thibault-Monnier]

@andykaylor @bcardosolopes I am done.

[andykaylor]

@andykaylor @bcardosolopes I am done.

@Thibault-Monnier It looks like it still has merge conflicts with the sse-builtins.c test, which will require rebasing.

[Thibault-Monnier]

Fixed

[andykaylor]

It looks like the CI build is failing now because it picked up the bad state from a previous commit. Can you rebase one more time?

[Thibault-Monnier]

Fixed, all checks are passing.