[MSAN] Handle x86 {round,min,max}sd intrinsics

[guiand]

Summary

These need special handling over the simple vector intrinsics as they behave more like a shuffle operation: taking the top half of the vector from one input, and the bottom half separately. Previously, these were being handled as though all bits of all operands were combined.

[guiand]

Ran clang-format

[eugenis]

The test should be in LLVM, under test/Instrumentation/MemorySanitizer

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

3077

You probably want to insert in First, not Second.

Is the generated code any better if you OR the vectors, and then shuffle to put the top element of First into the top element of the output? That's what LLVM generates if I express this logic in C.

[guiand]

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

3077

The codegen is basically identical either way, but if you'd like I can still upload a patch to change these into shufflevector instructions.

[guiand]

Use shufflevector, move test over to IR

[eugenis]

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

3077

This is much better.
Use makeArrayRef({2, 1}).

3331

Unrelated change.

llvm/test/Instrumentation/MemorySanitizer/vector_sd.ll

17 ↗

(On Diff #272842)

pass the vectors as arguments, it will make the test case a lot simpler

[guiand]

Addressed comments

[guiand]

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

3331

Whoops!

[eugenis]

LGTM

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

3077

llvm:: is unnecessary, and <int> is probably too

clang/test/CodeGen/msan-intrinsics.c

+// RUN: %clang_cc1 -fsanitize=memory -triple x86_64-linux-gnu -emit-llvm %s -O3 -o - | FileCheck %s
+
+typedef double double2 __attribute__((vector_size(16)));
+
+__attribute__((target("sse4.1")))
+double2
+RoundSD(double theta, double top) {
+  // CHECK: [[IN_SHADOW1:%[0-9]+]] = load {{.*}} @__msan_param_tls
+  // CHECK: [[IN_SHADOW2:%[0-9]+]] = load {{.*}} @__msan_param_tls
+  double2 vec;
+  double2 unused;
+  unused[1] = top;
+  vec[0] = theta;
+  return __builtin_ia32_roundsd(unused, vec, 1);
+  // CHECK: [[TMP_SHADOW:%.+]] = insertelement <2 x i64> undef, i64 [[IN_SHADOW2]], i32 1
+  // CHECK: [[OUT_SHADOW:%.+]] = insertelement <2 x i64> [[TMP_SHADOW]], i64 [[IN_SHADOW1]], i32 0
+
+  // CHECK-NOT: call void @msan_warning
+  // CHECK: call <2 x double> @llvm.x86.sse41.round.sd
+  // CHECK: store <2 x i64> [[OUT_SHADOW]], {{.*}} @__msan_retval_tls
+  // CHECK: ret <2 x double>
+}
+
+__attribute__((target("sse2"))) double MinSD(double t1, double t2) {
+  // CHECK: [[IN_SHADOW1:%[0-9]+]] = load {{.*}} @__msan_param_tls
+  // CHECK: [[IN_SHADOW2:%[0-9]+]] = load {{.*}} @__msan_param_tls
+  // CHECK: [[COMB_SHADOW:%[0-9]+]] = or i64 [[IN_SHADOW2]], [[IN_SHADOW1]]
+  double2 first;
+  double2 second;
+  first[0] = t1;
+  second[0] = t2;
+  double min = __builtin_ia32_minsd(first, second)[0];
+  // CHECK-NOT: call void @msan_warning
+  // CHECK: call <2 x double> @llvm.x86.sse2.min.sd
+  return min;
+  // CHECK: store i64 [[COMB_SHADOW]], {{.*}} @__msan_retval_tls
+  // CHECK: ret double
+}
 No newline at end of file

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

Value *Shuf1 =
         IRB.CreateShuffleVector(getShadow(&I, 1), UndefValue::get(ShadowTy),
                                 getPclmulMask(Width, Imm & 0x10));
     ShadowAndOriginCombiner SOC(this, IRB);
     SOC.Add(Shuf0, getOrigin(&I, 0));
     SOC.Add(Shuf1, getOrigin(&I, 1));
     SOC.Done(&I);
   }
 
+  // Instrument _mm_*_sd intrinsics
+  void handleUnarySdIntrinsic(IntrinsicInst &I) {
+    IRBuilder<> IRB(&I);
+    Value *First = getShadow(&I, 0);
+    Value *Second = getShadow(&I, 1);
+    Value *LowShadow =
+        IRB.CreateExtractElement(Second, IRB.getInt32(0), "_lo_a");
+    Value *Shadow = IRB.CreateInsertElement(First, LowShadow, IRB.getInt32(0));
+
+    setShadow(&I, Shadow);
+    setOriginForNaryOp(I);
+  }
+
+  void handleBinarySdIntrinsic(IntrinsicInst &I) {
+    IRBuilder<> IRB(&I);
+    Value *First = getShadow(&I, 0);
+    Value *Second = getShadow(&I, 1);
+    Value *LowA = IRB.CreateExtractElement(First, IRB.getInt32(0), "_lo_a");
+    Value *LowB = IRB.CreateExtractElement(Second, IRB.getInt32(0), "_lo_b");
+    Value *LowShadow = IRB.CreateOr(LowA, LowB);
+    Value *Shadow = IRB.CreateInsertElement(Second, LowShadow, IRB.getInt32(0));
+
+    setShadow(&I, Shadow);
+    setOriginForNaryOp(I);
+  }
+
   void visitIntrinsicInst(IntrinsicInst &I) {
     switch (I.getIntrinsicID()) {
     case Intrinsic::lifetime_start:
       handleLifetimeStart(I);
       break;
     case Intrinsic::launder_invariant_group:
     case Intrinsic::strip_invariant_group:
       handleInvariantGroup(I);
case Intrinsic::x86_bmi_pext_64:
       break;
 
     case Intrinsic::x86_pclmulqdq:
     case Intrinsic::x86_pclmulqdq_256:
     case Intrinsic::x86_pclmulqdq_512:
       handlePclmulIntrinsic(I);
       break;
 
+    case Intrinsic::x86_sse41_round_sd:
+      handleUnarySdIntrinsic(I);
+      break;
+    case Intrinsic::x86_sse2_max_sd:
+    case Intrinsic::x86_sse2_min_sd:
+      handleBinarySdIntrinsic(I);
+      break;
     case Intrinsic::is_constant:
       // The result of llvm.is.constant() is always defined.
       setShadow(&I, getCleanShadow(&I));
       setOrigin(&I, getCleanOrigin());
       break;
 
     default:
       if (!handleUnknownIntrinsic(I))
         visitInstruction(I);
       break;
     }