[msan] Generalize handlePairwiseShadowOrIntrinsic, and handle x86 pairwise add/sub #127567
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rwise add/sub x86 pairwise add and sub are currently handled by applying the pairwise add intrinsic to the shadow (llvm#124835), due to the lack of an x86 pairwise OR intrinsic. handlePairwiseShadowOrIntrinsic was added (llvm#126008) to handle Arm pairwise add, but assumes that the intrinsic operates on each pair of elements as defined by the LLVM type. In contrast, x86 pairwise add/sub may sometimes have e.g., <1 x i64> as a parameter but actually be operating on <2 x i32>. This patch generalizes handlePairwiseShadowOrIntrinsic, to allow reinterpreting the parameters to be a vector of specified element size, and then uses this function to handle x86 pairwise add/sub.
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-compiler-rt-sanitizer Author: Thurston Dang (thurstond) Changesx86 pairwise add and sub are currently handled by applying the pairwise add intrinsic to the shadow (#124835), due to the lack of an x86 pairwise OR intrinsic. handlePairwiseShadowOrIntrinsic was added (#126008) to handle Arm pairwise add, but assumes that the intrinsic operates on each pair of elements as defined by the LLVM type. In contrast, x86 pairwise add/sub may sometimes have e.g., <1 x i64> as a parameter but actually be operating on <2 x i32>. This patch generalizes handlePairwiseShadowOrIntrinsic, to allow reinterpreting the parameters to be a vector of specified element size, and then uses this function to handle x86 pairwise add/sub. Patch is 51.10 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/127567.diff 7 Files Affected:
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 8708489ac4fef..4ca3f0f3a3f8e 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -2608,8 +2608,15 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
/// e.g., <2 x i32> @llvm.aarch64.neon.saddlp.v2i32.v4i16(<4 x i16>)
/// <16 x i8> @llvm.aarch64.neon.addp.v16i8(<16 x i8>, <16 x i8>)
///
- /// TODO: adapt this function to handle horizontal add/sub?
- void handlePairwiseShadowOrIntrinsic(IntrinsicInst &I) {
+ /// Optionally, reinterpret the parameters to have elements of a specified
+ /// width. For example:
+ /// @llvm.x86.ssse3.phadd.w(<1 x i64> [[VAR1]], <1 x i64> [[VAR2]])
+ /// conceptually operates on
+ /// (<4 x i16> [[VAR1]], <4 x i16> [[VAR2]])
+ /// and can be handled with ReinterpretElemWidth == 16.
+ void
+ handlePairwiseShadowOrIntrinsic(IntrinsicInst &I,
+ std::optional<int> ReinterpretElemWidth) {
assert(I.arg_size() == 1 || I.arg_size() == 2);
assert(I.getType()->isVectorTy());
@@ -2618,28 +2625,56 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
FixedVectorType *ParamType =
cast<FixedVectorType>(I.getArgOperand(0)->getType());
if (I.arg_size() == 2)
- assert(ParamType == cast<FixedVectorType>(I.getArgOperand(1)->getType()));
+ assert(I.getArgOperand(0)->getType() == I.getArgOperand(1)->getType());
+
[[maybe_unused]] FixedVectorType *ReturnType =
cast<FixedVectorType>(I.getType());
assert(ParamType->getNumElements() * I.arg_size() ==
2 * ReturnType->getNumElements());
IRBuilder<> IRB(&I);
- unsigned Width = ParamType->getNumElements() * I.arg_size();
+
+ unsigned TotalNumElems = ParamType->getNumElements() * I.arg_size();
+ FixedVectorType *ReinterpretShadowTy = nullptr;
+ if (ReinterpretElemWidth.has_value()) {
+ assert(ParamType->getPrimitiveSizeInBits() %
+ ReinterpretElemWidth.value() ==
+ 0);
+ ReinterpretShadowTy = FixedVectorType::get(
+ IRB.getIntNTy(ReinterpretElemWidth.value()),
+ ParamType->getPrimitiveSizeInBits() / ReinterpretElemWidth.value());
+ TotalNumElems = ReinterpretShadowTy->getNumElements() * I.arg_size();
+ }
// Horizontal OR of shadow
SmallVector<int, 8> EvenMask;
SmallVector<int, 8> OddMask;
- for (unsigned X = 0; X < Width; X += 2) {
+ for (unsigned X = 0; X + 1 < TotalNumElems; X += 2) {
EvenMask.push_back(X);
OddMask.push_back(X + 1);
}
Value *FirstArgShadow = getShadow(&I, 0);
+ if (ReinterpretShadowTy)
+ FirstArgShadow = IRB.CreateBitCast(FirstArgShadow, ReinterpretShadowTy);
+
+ // If we had two parameters each with an odd number of elements, the total
+ // number of elements is even, but we have never seen this in extant
+ // instruction sets, so we enforce that each parameter must have an even
+ // number of elements.
+ assert(
+ (cast<FixedVectorType>(FirstArgShadow->getType())->getNumElements()) %
+ 2 ==
+ 0);
+
Value *EvenShadow;
Value *OddShadow;
if (I.arg_size() == 2) {
Value *SecondArgShadow = getShadow(&I, 1);
+ if (ReinterpretShadowTy)
+ SecondArgShadow =
+ IRB.CreateBitCast(SecondArgShadow, ReinterpretShadowTy);
+
EvenShadow =
IRB.CreateShuffleVector(FirstArgShadow, SecondArgShadow, EvenMask);
OddShadow =
@@ -2653,6 +2688,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
OrShadow = CreateShadowCast(IRB, OrShadow, getShadowTy(&I));
setShadow(&I, OrShadow);
+
setOriginForNaryOp(I);
}
@@ -4156,87 +4192,6 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
setOriginForNaryOp(I);
}
- void handleAVXHorizontalAddSubIntrinsic(IntrinsicInst &I) {
- // Approximation only:
- // output = horizontal_add/sub(A, B)
- // => shadow[output] = horizontal_add(shadow[A], shadow[B])
- //
- // We always use horizontal add instead of subtract, because subtracting
- // a fully uninitialized shadow would result in a fully initialized shadow.
- //
- // - If we add two adjacent zero (initialized) shadow values, the
- // result always be zero i.e., no false positives.
- // - If we add two shadows, one of which is uninitialized, the
- // result will always be non-zero i.e., no false negatives.
- // - However, we can have false negatives if we do an addition that wraps
- // to zero; we consider this an acceptable tradeoff for performance.
- //
- // To make shadow propagation precise, we want the equivalent of
- // "horizontal OR", but this is not available for SSE3/SSSE3/AVX/AVX2.
-
- Intrinsic::ID shadowIntrinsicID = I.getIntrinsicID();
-
- switch (I.getIntrinsicID()) {
- case Intrinsic::x86_sse3_hsub_ps:
- shadowIntrinsicID = Intrinsic::x86_sse3_hadd_ps;
- break;
-
- case Intrinsic::x86_sse3_hsub_pd:
- shadowIntrinsicID = Intrinsic::x86_sse3_hadd_pd;
- break;
-
- case Intrinsic::x86_ssse3_phsub_d:
- shadowIntrinsicID = Intrinsic::x86_ssse3_phadd_d;
- break;
-
- case Intrinsic::x86_ssse3_phsub_d_128:
- shadowIntrinsicID = Intrinsic::x86_ssse3_phadd_d_128;
- break;
-
- case Intrinsic::x86_ssse3_phsub_w:
- shadowIntrinsicID = Intrinsic::x86_ssse3_phadd_w;
- break;
-
- case Intrinsic::x86_ssse3_phsub_w_128:
- shadowIntrinsicID = Intrinsic::x86_ssse3_phadd_w_128;
- break;
-
- case Intrinsic::x86_ssse3_phsub_sw:
- shadowIntrinsicID = Intrinsic::x86_ssse3_phadd_sw;
- break;
-
- case Intrinsic::x86_ssse3_phsub_sw_128:
- shadowIntrinsicID = Intrinsic::x86_ssse3_phadd_sw_128;
- break;
-
- case Intrinsic::x86_avx_hsub_pd_256:
- shadowIntrinsicID = Intrinsic::x86_avx_hadd_pd_256;
- break;
-
- case Intrinsic::x86_avx_hsub_ps_256:
- shadowIntrinsicID = Intrinsic::x86_avx_hadd_ps_256;
- break;
-
- case Intrinsic::x86_avx2_phsub_d:
- shadowIntrinsicID = Intrinsic::x86_avx2_phadd_d;
- break;
-
- case Intrinsic::x86_avx2_phsub_w:
- shadowIntrinsicID = Intrinsic::x86_avx2_phadd_w;
- break;
-
- case Intrinsic::x86_avx2_phsub_sw:
- shadowIntrinsicID = Intrinsic::x86_avx2_phadd_sw;
- break;
-
- default:
- break;
- }
-
- return handleIntrinsicByApplyingToShadow(I, shadowIntrinsicID,
- /*trailingVerbatimArgs*/ 0);
- }
-
/// Handle Arm NEON vector store intrinsics (vst{2,3,4}, vst1x_{2,3,4},
/// and vst{2,3,4}lane).
///
@@ -4783,33 +4738,49 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
handleVtestIntrinsic(I);
break;
- case Intrinsic::x86_sse3_hadd_ps:
- case Intrinsic::x86_sse3_hadd_pd:
- case Intrinsic::x86_ssse3_phadd_d:
- case Intrinsic::x86_ssse3_phadd_d_128:
+ // Packed Horizontal Add/Subtract
case Intrinsic::x86_ssse3_phadd_w:
case Intrinsic::x86_ssse3_phadd_w_128:
+ case Intrinsic::x86_avx2_phadd_w:
+ case Intrinsic::x86_ssse3_phsub_w:
+ case Intrinsic::x86_ssse3_phsub_w_128:
+ case Intrinsic::x86_avx2_phsub_w: {
+ handlePairwiseShadowOrIntrinsic(I, /*ReinterpretElemWidth=*/16);
+ break;
+ }
+
+ // Packed Horizontal Add/Subtract
+ case Intrinsic::x86_ssse3_phadd_d:
+ case Intrinsic::x86_ssse3_phadd_d_128:
+ case Intrinsic::x86_avx2_phadd_d:
+ case Intrinsic::x86_ssse3_phsub_d:
+ case Intrinsic::x86_ssse3_phsub_d_128:
+ case Intrinsic::x86_avx2_phsub_d: {
+ handlePairwiseShadowOrIntrinsic(I, /*ReinterpretElemWidth=*/32);
+ break;
+ }
+
+ // Packed Horizontal Add/Subtract and Saturate
case Intrinsic::x86_ssse3_phadd_sw:
case Intrinsic::x86_ssse3_phadd_sw_128:
+ case Intrinsic::x86_avx2_phadd_sw:
+ case Intrinsic::x86_ssse3_phsub_sw:
+ case Intrinsic::x86_ssse3_phsub_sw_128:
+ case Intrinsic::x86_avx2_phsub_sw: {
+ handlePairwiseShadowOrIntrinsic(I, /*ReinterpretElemWidth=*/16);
+ break;
+ }
+
+ // Packed Single/Double Precision Floating-Point Horizontal Add
+ case Intrinsic::x86_sse3_hadd_ps:
+ case Intrinsic::x86_sse3_hadd_pd:
case Intrinsic::x86_avx_hadd_pd_256:
case Intrinsic::x86_avx_hadd_ps_256:
- case Intrinsic::x86_avx2_phadd_d:
- case Intrinsic::x86_avx2_phadd_w:
- case Intrinsic::x86_avx2_phadd_sw:
case Intrinsic::x86_sse3_hsub_ps:
case Intrinsic::x86_sse3_hsub_pd:
- case Intrinsic::x86_ssse3_phsub_d:
- case Intrinsic::x86_ssse3_phsub_d_128:
- case Intrinsic::x86_ssse3_phsub_w:
- case Intrinsic::x86_ssse3_phsub_w_128:
- case Intrinsic::x86_ssse3_phsub_sw:
- case Intrinsic::x86_ssse3_phsub_sw_128:
case Intrinsic::x86_avx_hsub_pd_256:
- case Intrinsic::x86_avx_hsub_ps_256:
- case Intrinsic::x86_avx2_phsub_d:
- case Intrinsic::x86_avx2_phsub_w:
- case Intrinsic::x86_avx2_phsub_sw: {
- handleAVXHorizontalAddSubIntrinsic(I);
+ case Intrinsic::x86_avx_hsub_ps_256: {
+ handlePairwiseShadowOrIntrinsic(I, /*ReinterpretElemWidth=*/std::nullopt);
break;
}
@@ -4869,7 +4840,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
// Add Long Pairwise
case Intrinsic::aarch64_neon_saddlp:
case Intrinsic::aarch64_neon_uaddlp: {
- handlePairwiseShadowOrIntrinsic(I);
+ handlePairwiseShadowOrIntrinsic(I, std::nullopt);
break;
}
diff --git a/llvm/test/Instrumentation/MemorySanitizer/X86/avx-intrinsics-x86.ll b/llvm/test/Instrumentation/MemorySanitizer/X86/avx-intrinsics-x86.ll
index 26e9c39696f70..d85ab2c2c4bad 100644
--- a/llvm/test/Instrumentation/MemorySanitizer/X86/avx-intrinsics-x86.ll
+++ b/llvm/test/Instrumentation/MemorySanitizer/X86/avx-intrinsics-x86.ll
@@ -435,10 +435,9 @@ define <4 x double> @test_x86_avx_hadd_pd_256(<4 x double> %a0, <4 x double> %a1
; CHECK-NEXT: [[TMP1:%.*]] = load <4 x i64>, ptr @__msan_param_tls, align 8
; CHECK-NEXT: [[TMP2:%.*]] = load <4 x i64>, ptr inttoptr (i64 add (i64 ptrtoint (ptr @__msan_param_tls to i64), i64 32) to ptr), align 8
; CHECK-NEXT: call void @llvm.donothing()
-; CHECK-NEXT: [[A0:%.*]] = bitcast <4 x i64> [[TMP1]] to <4 x double>
-; CHECK-NEXT: [[A1:%.*]] = bitcast <4 x i64> [[TMP2]] to <4 x double>
-; CHECK-NEXT: [[RES:%.*]] = call <4 x double> @llvm.x86.avx.hadd.pd.256(<4 x double> [[A0]], <4 x double> [[A1]])
-; CHECK-NEXT: [[_MSPROP:%.*]] = bitcast <4 x double> [[RES]] to <4 x i64>
+; CHECK-NEXT: [[TMP3:%.*]] = shufflevector <4 x i64> [[TMP1]], <4 x i64> [[TMP2]], <4 x i32> <i32 0, i32 2, i32 4, i32 6>
+; CHECK-NEXT: [[TMP4:%.*]] = shufflevector <4 x i64> [[TMP1]], <4 x i64> [[TMP2]], <4 x i32> <i32 1, i32 3, i32 5, i32 7>
+; CHECK-NEXT: [[_MSPROP:%.*]] = or <4 x i64> [[TMP3]], [[TMP4]]
; CHECK-NEXT: [[RES1:%.*]] = call <4 x double> @llvm.x86.avx.hadd.pd.256(<4 x double> [[A2:%.*]], <4 x double> [[A3:%.*]])
; CHECK-NEXT: store <4 x i64> [[_MSPROP]], ptr @__msan_retval_tls, align 8
; CHECK-NEXT: ret <4 x double> [[RES1]]
@@ -454,10 +453,9 @@ define <8 x float> @test_x86_avx_hadd_ps_256(<8 x float> %a0, <8 x float> %a1) #
; CHECK-NEXT: [[TMP1:%.*]] = load <8 x i32>, ptr @__msan_param_tls, align 8
; CHECK-NEXT: [[TMP2:%.*]] = load <8 x i32>, ptr inttoptr (i64 add (i64 ptrtoint (ptr @__msan_param_tls to i64), i64 32) to ptr), align 8
; CHECK-NEXT: call void @llvm.donothing()
-; CHECK-NEXT: [[A0:%.*]] = bitcast <8 x i32> [[TMP1]] to <8 x float>
-; CHECK-NEXT: [[A1:%.*]] = bitcast <8 x i32> [[TMP2]] to <8 x float>
-; CHECK-NEXT: [[RES:%.*]] = call <8 x float> @llvm.x86.avx.hadd.ps.256(<8 x float> [[A0]], <8 x float> [[A1]])
-; CHECK-NEXT: [[_MSPROP:%.*]] = bitcast <8 x float> [[RES]] to <8 x i32>
+; CHECK-NEXT: [[TMP3:%.*]] = shufflevector <8 x i32> [[TMP1]], <8 x i32> [[TMP2]], <8 x i32> <i32 0, i32 2, i32 4, i32 6, i32 8, i32 10, i32 12, i32 14>
+; CHECK-NEXT: [[TMP4:%.*]] = shufflevector <8 x i32> [[TMP1]], <8 x i32> [[TMP2]], <8 x i32> <i32 1, i32 3, i32 5, i32 7, i32 9, i32 11, i32 13, i32 15>
+; CHECK-NEXT: [[_MSPROP:%.*]] = or <8 x i32> [[TMP3]], [[TMP4]]
; CHECK-NEXT: [[RES1:%.*]] = call <8 x float> @llvm.x86.avx.hadd.ps.256(<8 x float> [[A2:%.*]], <8 x float> [[A3:%.*]])
; CHECK-NEXT: store <8 x i32> [[_MSPROP]], ptr @__msan_retval_tls, align 8
; CHECK-NEXT: ret <8 x float> [[RES1]]
@@ -473,10 +471,9 @@ define <4 x double> @test_x86_avx_hsub_pd_256(<4 x double> %a0, <4 x double> %a1
; CHECK-NEXT: [[TMP1:%.*]] = load <4 x i64>, ptr @__msan_param_tls, align 8
; CHECK-NEXT: [[TMP2:%.*]] = load <4 x i64>, ptr inttoptr (i64 add (i64 ptrtoint (ptr @__msan_param_tls to i64), i64 32) to ptr), align 8
; CHECK-NEXT: call void @llvm.donothing()
-; CHECK-NEXT: [[A0:%.*]] = bitcast <4 x i64> [[TMP1]] to <4 x double>
-; CHECK-NEXT: [[A1:%.*]] = bitcast <4 x i64> [[TMP2]] to <4 x double>
-; CHECK-NEXT: [[RES:%.*]] = call <4 x double> @llvm.x86.avx.hadd.pd.256(<4 x double> [[A0]], <4 x double> [[A1]])
-; CHECK-NEXT: [[_MSPROP:%.*]] = bitcast <4 x double> [[RES]] to <4 x i64>
+; CHECK-NEXT: [[TMP3:%.*]] = shufflevector <4 x i64> [[TMP1]], <4 x i64> [[TMP2]], <4 x i32> <i32 0, i32 2, i32 4, i32 6>
+; CHECK-NEXT: [[TMP4:%.*]] = shufflevector <4 x i64> [[TMP1]], <4 x i64> [[TMP2]], <4 x i32> <i32 1, i32 3, i32 5, i32 7>
+; CHECK-NEXT: [[_MSPROP:%.*]] = or <4 x i64> [[TMP3]], [[TMP4]]
; CHECK-NEXT: [[RES1:%.*]] = call <4 x double> @llvm.x86.avx.hsub.pd.256(<4 x double> [[A2:%.*]], <4 x double> [[A3:%.*]])
; CHECK-NEXT: store <4 x i64> [[_MSPROP]], ptr @__msan_retval_tls, align 8
; CHECK-NEXT: ret <4 x double> [[RES1]]
@@ -492,10 +489,9 @@ define <8 x float> @test_x86_avx_hsub_ps_256(<8 x float> %a0, <8 x float> %a1) #
; CHECK-NEXT: [[TMP1:%.*]] = load <8 x i32>, ptr @__msan_param_tls, align 8
; CHECK-NEXT: [[TMP2:%.*]] = load <8 x i32>, ptr inttoptr (i64 add (i64 ptrtoint (ptr @__msan_param_tls to i64), i64 32) to ptr), align 8
; CHECK-NEXT: call void @llvm.donothing()
-; CHECK-NEXT: [[A0:%.*]] = bitcast <8 x i32> [[TMP1]] to <8 x float>
-; CHECK-NEXT: [[A1:%.*]] = bitcast <8 x i32> [[TMP2]] to <8 x float>
-; CHECK-NEXT: [[RES:%.*]] = call <8 x float> @llvm.x86.avx.hadd.ps.256(<8 x float> [[A0]], <8 x float> [[A1]])
-; CHECK-NEXT: [[_MSPROP:%.*]] = bitcast <8 x float> [[RES]] to <8 x i32>
+; CHECK-NEXT: [[TMP3:%.*]] = shufflevector <8 x i32> [[TMP1]], <8 x i32> [[TMP2]], <8 x i32> <i32 0, i32 2, i32 4, i32 6, i32 8, i32 10, i32 12, i32 14>
+; CHECK-NEXT: [[TMP4:%.*]] = shufflevector <8 x i32> [[TMP1]], <8 x i32> [[TMP2]], <8 x i32> <i32 1, i32 3, i32 5, i32 7, i32 9, i32 11, i32 13, i32 15>
+; CHECK-NEXT: [[_MSPROP:%.*]] = or <8 x i32> [[TMP3]], [[TMP4]]
; CHECK-NEXT: [[RES1:%.*]] = call <8 x float> @llvm.x86.avx.hsub.ps.256(<8 x float> [[A2:%.*]], <8 x float> [[A3:%.*]])
; CHECK-NEXT: store <8 x i32> [[_MSPROP]], ptr @__msan_retval_tls, align 8
; CHECK-NEXT: ret <8 x float> [[RES1]]
diff --git a/llvm/test/Instrumentation/MemorySanitizer/X86/avx2-intrinsics-x86.ll b/llvm/test/Instrumentation/MemorySanitizer/X86/avx2-intrinsics-x86.ll
index 5597a9c96611f..f916130fe53e5 100644
--- a/llvm/test/Instrumentation/MemorySanitizer/X86/avx2-intrinsics-x86.ll
+++ b/llvm/test/Instrumentation/MemorySanitizer/X86/avx2-intrinsics-x86.ll
@@ -569,7 +569,9 @@ define <8 x i32> @test_x86_avx2_phadd_d(<8 x i32> %a0, <8 x i32> %a1) #0 {
; CHECK-NEXT: [[TMP1:%.*]] = load <8 x i32>, ptr @__msan_param_tls, align 8
; CHECK-NEXT: [[TMP2:%.*]] = load <8 x i32>, ptr inttoptr (i64 add (i64 ptrtoint (ptr @__msan_param_tls to i64), i64 32) to ptr), align 8
; CHECK-NEXT: call void @llvm.donothing()
-; CHECK-NEXT: [[_MSPROP:%.*]] = call <8 x i32> @llvm.x86.avx2.phadd.d(<8 x i32> [[TMP1]], <8 x i32> [[TMP2]])
+; CHECK-NEXT: [[TMP9:%.*]] = shufflevector <8 x i32> [[TMP1]], <8 x i32> [[TMP2]], <8 x i32> <i32 0, i32 2, i32 4, i32 6, i32 8, i32 10, i32 12, i32 14>
+; CHECK-NEXT: [[TMP10:%.*]] = shufflevector <8 x i32> [[TMP1]], <8 x i32> [[TMP2]], <8 x i32> <i32 1, i32 3, i32 5, i32 7, i32 9, i32 11, i32 13, i32 15>
+; CHECK-NEXT: [[_MSPROP:%.*]] = or <8 x i32> [[TMP9]], [[TMP10]]
; CHECK-NEXT: [[RES:%.*]] = call <8 x i32> @llvm.x86.avx2.phadd.d(<8 x i32> [[A0:%.*]], <8 x i32> [[A1:%.*]])
; CHECK-NEXT: store <8 x i32> [[_MSPROP]], ptr @__msan_retval_tls, align 8
; CHECK-NEXT: ret <8 x i32> [[RES]]
@@ -585,7 +587,9 @@ define <16 x i16> @test_x86_avx2_phadd_sw(<16 x i16> %a0, <16 x i16> %a1) #0 {
; CHECK-NEXT: [[TMP1:%.*]] = load <16 x i16>, ptr @__msan_param_tls, align 8
; CHECK-NEXT: [[TMP2:%.*]] = load <16 x i16>, ptr inttoptr (i64 add (i64 ptrtoint (ptr @__msan_param_tls to i64), i64 32) to ptr), align 8
; CHECK-NEXT: call void @llvm.donothing()
-; CHECK-NEXT: [[_MSPROP:%.*]] = call <16 x i16> @llvm.x86.avx2.phadd.sw(<16 x i16> [[TMP1]], <16 x i16> [[TMP2]])
+; CHECK-NEXT: [[TMP9:%.*]] = shufflevector <16 x i16> [[TMP1]], <16 x i16> [[TMP2]], <16 x i32> <i32 0, i32 2, i32 4, i32 6, i32 8, i32 10, i32 12, i32 14, i32 16, i32 18, i32 20, i32 22, i32 24, i32 26, i32 28, i32 30>
+; CHECK-NEXT: [[TMP10:%.*]] = shufflevector <16 x i16> [[TMP1]], <16 x i16> [[TMP2]], <16 x i32> <i32 1, i32 3, i32 5, i32 7, i32 9, i32 11, i32 13, i32 15, i32 17, i32 19, i32 21, i32 23, i32 25, i32 27, i32 29, i32 31>
+; CHECK-NEXT: [[_MSPROP:%.*]] = or <16 x i16> [[TMP9]], [[TMP10]]
; CHECK-NEXT: [[RES:%.*]] = call <16 x i16> @llvm.x86.avx2.phadd.sw(<16 x i16> [[A0:%.*]], <16 x i16> [[A1:%.*]])
; CHECK-NEXT: store <16 x i16> [[_MSPROP]], ptr @__msan_retval_tls, align 8
; CHECK-NEXT: ret <16 x i16> [[RES]]
@@ -601,7 +605,9 @@ define <16 x i16> @test_x86_avx2_phadd_w(<16 x i16> %a0, <16 x i16> %a1) #0 {
; CHECK-NEXT: [[TMP1:%.*]] = load <16 x i16>, ptr @__msan_param_tls, align 8
; CHECK-NEXT: [[TMP2:%.*]] = load <16 x i16>, ptr inttoptr (i64 add (i64 ptrtoint (ptr @__msan_param_tls to i64), i64 32) to ptr), align 8
; CHECK-NEXT: call void @llvm.donothing()
-; CHECK-NEXT: [[_MSPROP:%.*]] = call <16 x i16> @llvm.x86.avx2.phadd.w(<16 x i16> [[TMP1]], <16 x i16> [[TMP2]])
+; CHECK-NEXT: [[TMP9:%.*]] = shufflevector <16 x i16> [[TMP1]], <16 x i16> [[TMP2]], <16 x i32> <i32 0, i32 2, i32 4, i32 6, i32 8, i32 10, i32 12, i32 14, i32 16, i32 18, i32 20, i32 22, i32 24, i32 26, i32 28, i32 30>
+; CHECK-NEXT: [[TMP10:%.*]] = shufflevector <16 x i16> [[TMP1]], <16 x i16> [[TMP2]], <16 x i32> <i32 1, i32 3, i32 5, i32 7, i32 9, i32 11, i32 13, i32 15, i32 17, i32 19, i32 21, i32 23, i32 25, i32 27, i32 29, i32 31>
+; CHECK-NEXT: [[_MSPROP:%.*]] = or <16 x i16> [[TMP9]], [[TMP10]]
; CHECK-NEXT: [[RES:%.*]] = call <16 x i16> @llvm.x86.avx2.phadd.w(<16 x i16> [[A0:%.*]], <16 x i16> [[A1:%.*]])
; CHECK-NEXT: store <16 x i16> [[_MSPROP]], ptr @__msan_retval_tls, align 8
; CHECK-NEXT: ret <16 x i16> [[RES]]
@@ -617,7 +623,9 @@ define <8 x i32> @test_x86_avx2_phsub_d(<8 x i32> %a0, <8 x i32> %a1) #0 {
; CHECK-NEXT: [[TMP1:%.*]] = load <8 x i32>, ptr @__msan_param_tls, align 8
; CHECK-NEXT: [[TMP2:%.*]] = load <8 x i32>, ptr inttoptr (i64 add (i64 ptrtoint (ptr @__msan_param_tls to i64), i64 32) to ptr), align 8
; CHECK-NEXT: call void @llvm.donothing()
-; CHECK-NEXT: [[_MSPROP:%.*]] = call <8 x i32> @llvm.x86.avx2.phadd.d(<8 x i32> [[TMP1]], <8 x i32> [[TMP2]])
+; CHECK-NEXT: [[TMP9:%.*]] = shufflevector <8 x i32> [[TMP1]], <8 x i32> [[TMP2]], <8 x i32> <i32 0, i32 2, i32 4, i32 6, i32 8, i32 10, i32 12, i32 14>
+; CHECK-NEXT: [[TMP10:%.*]] = shufflevector <8 x i32> [[TMP1]], <8 x i32> [[TMP2]], <8 x i32> <i32 1, i32 3, i32 5, i32 7, i32 9, i32 11, i32 13, i32 15>
+; CHECK-NEXT: [[_MSPROP:%.*]] = or <8 x i32> [[TMP9]], [[TMP10]]
; CHECK-NEXT: [[RES:%.*]] = call <8 x i32> @llvm.x86.avx2.phsub.d(<8 x i32> [[A0:%.*]], <8 x i32> [[A1:%.*]])
; CHECK-NEXT: store <8 x i32> [[_MSPROP]], ptr @__msan_retval_tls, align 8
; CHECK-NEXT: ret <8 x i32> [[RES]]
@@ -633,7 +641,9 @@ define <16 x i16> @test_x86_avx2_phsub_sw(<16 x i16> %a0, <16 x i16> %a1...
[truncated]
|
fmayer
reviewed
Feb 18, 2025
| @@ -2653,6 +2688,7 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> { | |||
| OrShadow = CreateShadowCast(IRB, OrShadow, getShadowTy(&I)); | |||
|
|
|||
| setShadow(&I, OrShadow); | |||
|
|
|||
|
|
||
| // Horizontal OR of shadow | ||
| SmallVector<int, 8> EvenMask; | ||
| SmallVector<int, 8> OddMask; | ||
| for (unsigned X = 0; X < Width; X += 2) { | ||
| for (unsigned X = 0; X + 1 < TotalNumElems; X += 2) { |
There was a problem hiding this comment.
IMO, X < TotalNumElems - 1 is more standard form
| // instruction sets, so we enforce that each parameter must have an even | ||
| // number of elements. | ||
| assert( | ||
| (cast<FixedVectorType>(FirstArgShadow->getType())->getNumElements()) % |
There was a problem hiding this comment.
Done. Note I used LLVM's isAligned because sanitizer_common isn't included in MemorySanitizer.cpp.
| unsigned TotalNumElems = ParamType->getNumElements() * I.arg_size(); | ||
| FixedVectorType *ReinterpretShadowTy = nullptr; | ||
| if (ReinterpretElemWidth.has_value()) { | ||
| assert(ParamType->getPrimitiveSizeInBits() % |
There was a problem hiding this comment.
Done. Note I used LLVM's isAligned because sanitizer_common isn't included in MemorySanitizer.cpp.
| FixedVectorType *ReinterpretShadowTy = nullptr; | ||
| if (ReinterpretElemWidth.has_value()) { | ||
| assert(ParamType->getPrimitiveSizeInBits() % | ||
| ReinterpretElemWidth.value() == |
There was a problem hiding this comment.
IMO the value calls create clutter. I would recommend using *
vitalybuka
reviewed
Feb 20, 2025
| @@ -2618,28 +2625,54 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> { | |||
| FixedVectorType *ParamType = | |||
| cast<FixedVectorType>(I.getArgOperand(0)->getType()); | |||
| if (I.arg_size() == 2) | |||
There was a problem hiding this comment.
assert(I.arg_size() != 2 || I.getArgOperand(0)->getType() == I.getArgOperand(1)->getType());
vitalybuka
reviewed
Feb 20, 2025
| /// (<4 x i16> [[VAR1]], <4 x i16> [[VAR2]]) | ||
| /// and can be handled with ReinterpretElemWidth == 16. | ||
| void | ||
| handlePairwiseShadowOrIntrinsic(IntrinsicInst &I, |
There was a problem hiding this comment.
To many changes, could be better to fork this function for ReinterpretElemWidth case
|
TODO (can't convert to draft on gBus) |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
vitalybuka
approved these changes
Feb 27, 2025
joaosaffran
pushed a commit
to joaosaffran/llvm-project
that referenced
this pull request
Mar 3, 2025
…rwise add/sub (llvm#127567) x86 pairwise add and sub are currently handled by applying the pairwise add intrinsic to the shadow (llvm#124835), due to the lack of an x86 pairwise OR intrinsic. handlePairwiseShadowOrIntrinsic was added (llvm#126008) to handle Arm pairwise add, but assumes that the intrinsic operates on each pair of elements as defined by the LLVM type. In contrast, x86 pairwise add/sub may sometimes have e.g., <1 x i64> as a parameter but actually be operating on <2 x i32>. This patch generalizes handlePairwiseShadowOrIntrinsic, to allow reinterpreting the parameters to be a vector of specified element size, and then uses this function to handle x86 pairwise add/sub.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
x86 pairwise add and sub are currently handled by applying the pairwise add intrinsic to the shadow (#124835), due to the lack of an x86 pairwise OR intrinsic. handlePairwiseShadowOrIntrinsic was added (#126008) to handle Arm pairwise add, but assumes that the intrinsic operates on each pair of elements as defined by the LLVM type. In contrast, x86 pairwise add/sub may sometimes have e.g., <1 x i64> as a parameter but actually be operating on <2 x i32>.
This patch generalizes handlePairwiseShadowOrIntrinsic, to allow reinterpreting the parameters to be a vector of specified element size, and then uses this function to handle x86 pairwise add/sub.