[PATCH] D47686: [InstCombine] refine UB-handling in shuffle-binop transform

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:1427-1428
+      BinaryOperator::BinaryOps Opcode = Inst.getOpcode();
+      if (Opcode == Instruction::UDiv || Opcode == Instruction::URem ||
+          Opcode == Instruction::SDiv || Opcode == Instruction::SRem)
+        for (unsigned i = 0; i < VWidth; ++i)
----------------
lebedev.ri wrote:
> No special-handling for `fdiv`/`frem`?
No - this was clarified recently following the llvm-dev discussion about FP undef:
D44216


================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:1431
+          if (isa<UndefValue>(NewVecC[i]))
+            NewVecC[i] = ConstantInt::get(Inst.getType()->getScalarType(), 1);
 
----------------
lebedev.ri wrote:
> We want `int(1)` even for `float`s?
We're only changing integer opcodes/operands here. I can assert that if it would make the code clearer?


================
Comment at: test/Transforms/InstCombine/vec_shuffle.ll:855
 ; CHECK-LABEL: @fsub_splat_constant1(
-; CHECK-NEXT:    [[TMP1:%.*]] = fadd <2 x float> [[X:%.*]], <float -4.200000e+01, float -4.200000e+01>
+; CHECK-NEXT:    [[TMP1:%.*]] = fadd <2 x float> [[X:%.*]], <float -4.200000e+01, float 0x7FF8000000000000>
 ; CHECK-NEXT:    [[R:%.*]] = shufflevector <2 x float> [[TMP1]], <2 x float> undef, <2 x i32> zeroinitializer
----------------
lebedev.ri wrote:
> This looks like a float `NAN`?
> 
Yes. Interesting - I didn't notice that test was different than the rest of the FP cases!

So what happens here:
1. This transform puts in an undef element in the constant vector.
2. We canonicalize fsub with constant operand 1 to fadd:
  // X - C --> X + (-C)

```

  IC: Visiting:   %1 = fsub <2 x float> %x, <float 4.200000e+01, float undef>
  IC: Old =   %1 = fsub <2 x float> %x, <float 4.200000e+01, float undef>
      New =   <badref> = fadd <2 x float> %x, <float -4.200000e+01, float 0x7FF8000000000000>

```
...and in that constant conversion, we constant fold the undef vector element to a NaN constant.


https://reviews.llvm.org/D47686