[VPlan] Fix miscompile after PR #142433.

[jyknight]

This fixes a bug introduced by aa24029, "[VPlan] Unroll VPReplicateRecipe by VF", which cloned a VPReplicateRecipe without transferring the flags from the original.

That can cause incorrect nsw/nuw flags to be emitted on the new instructions, which may result in miscompiles.

It turns out there were no test-cases in the repo which end up hitting the situation where the recipe requires instruction clones to have different flags from the underlying instruction. The existing tests covered the flags being correct when the replacement instruction is a vectorized version of the initial instruction, but not when it required clones. A new test is added covering this.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-vectorizers

Author: James Y Knight (jyknight)

Changes

This fixes aa24029, "[VPlan] Unroll VPReplicateRecipe by VF", which cloned a VPReplicateRecipe without transferring the flags from the original.

That can cause incorrect nsw/nuw flags to be emitted on the new instructions, which may result in miscompiles.

It turns out there are currently zero test-cases in the repo which end up hitting the case where the recipe requires the clones to have different flags from the underlying instruction when cloning here, so I've also added a new test case minimized from the production code that was broken by this. Probably more extensive tests are needed in this area.

---

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

1 Files Affected:

• (modified) llvm/lib/Transforms/Vectorize/VPlanUnroll.cpp (+1)

diff --git a/llvm/lib/Transforms/Vectorize/VPlanUnroll.cpp b/llvm/lib/Transforms/Vectorize/VPlanUnroll.cpp
index 2dd43c092ff7a..b89cd21595efd 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanUnroll.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanUnroll.cpp
@@ -486,6 +486,7 @@ static VPReplicateRecipe *cloneForLane(VPlan &Plan, VPBuilder &Builder,
   auto *New =
       new VPReplicateRecipe(RepR->getUnderlyingInstr(), NewOps,
                             /*IsSingleScalar=*/true, /*Mask=*/nullptr, *RepR);
+  New->transferFlags(*RepR);
   New->insertBefore(RepR);
   return New;
 }

[fhahn]

Could you add the test to llvm/test/Transforms/LoopVectorize/X86/drop-poison-generating-flags.ll which already has coverage for dropping poison generating flags?

[jyknight]

Upon reviewing this test file, I found that my test case is almost the same as drop_vector_nuw_nsw -- except that I'm building with AVX instead of AVX512. I've replaced my test with a copy of that test, but built using AVX.

[jyknight]

PTAL, thanks!

[jyknight]

Upon reviewing this test file, I found that my test case is almost the same as drop_vector_nuw_nsw -- except that I'm building with AVX instead of AVX512. I've replaced my test with a copy of that test, but built using AVX.

[jyknight]

Given that the comments are only on the test, and this is fixing a regression, I'm going to go ahead and merge after having resolved the comments as far as I can tell. If you have any additional comments, I'll happily fix with a follow-up.