[PATCH] D84250: [InstSimplify] ExpandBinOp should fold an expression only when it's safe

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

I tried to find other examples where this could happen, and I can't find anything else either.
So I think this just needs some minor changes.
However, Alive2 does not show the "f_dontfold" test example or my reduced candidate test as incorrect - is that a bug for Alive2?
https://alive2.llvm.org/ce/z/dbGLzN



================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:231
 
+// CanFoldToConstantUsingOneOperand returns a constant after folding "L Op R"
+// after using only one of L or R, but not both.
----------------
Can say "This function returns..." to make it independent of the actual function name.
Use "///" to create documentation comment.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:249
+//      is 0 regardless of R.
+static Constant *FoldUsingOneOperand(Instruction::BinaryOps Op, Value *L,
+                                     Value *R) {
----------------
Formatting for function name: "Fold..." -> "fold..."


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:255-258
+  switch (Op) {
+  case Instruction::Shl:
+  case Instruction::AShr:
+  case Instruction::LShr:
----------------
Is this dead code? The current callers of expandBinOp only have "OpcodeToExpand" possiblities for:
Instruction::Add
Instruction::And
Instruction::Or
Instruction::Xor

If that is correct, maybe we want to just make it a 'TODO' comment?
If not, we could still simplify the code to something like:
  if (BinaryOperator::isShift(Op) && match(L, m_Zero()))



================
Comment at: llvm/test/Transforms/InstSimplify/distribute.ll:4
 
+; A transformation (x op y) op' z -> (x op' z) op (y op' z) is incorrect in
+; general. After z is copied, each instance of undefs in z may be instantiated
----------------
I was confused by this example because the 'x' in the code comment is not the same as the 'x' in the test.
How about reducing the example to:

```

define i4 @expand_and_xor(i4 %x, i4 %y) {
  %notx = xor i4 %x, -1
  %a = and i4 %notx, %y
  %nota = xor i4 %a, -1
  %result = and i4 %x, %nota
  ret i4 %result
}

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84250