[PATCH] D126877: [SLP]Fix PR55734: SLP vectorizer's reduce_and formation introduces poison.

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:11096
+      // (both operands are originally RHS), emit an extra freeze instruction
+      // for the LHS operand.
+      auto &&FixBoolLogicalOps = [&Builder](Value *&LHS, Value *&RHS,
----------------
This could use a comment to describe the patterns/logic. 

IIUC, we want to create a bool logic op like:
  select i1 LHS, i1 RHS, i1 false ; logical and

If we have original code like this:
  RedOp1 = select i1 ?, i1 LHS, i1 false
  RedOp2 = select i1 RHS, i1 ?, i1 false

Then, we swap LHS/RHS to create a new op that matches the poison semantics of the original code.

If we have original code like this and both values could be poison:
  RedOp1 = select i1 ?, i1 LHS, i1 false
  RedOp2 = select i1 ?, i1 RHS, i1 false

Then, we must freeze LHS in the new op. Can we manufacture a test for this 2nd path? I don't think there's any coverage for that path in the current test diffs.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126877/new/

https://reviews.llvm.org/D126877