[PATCH] D69038: [InstCombine] Fix miscompile bug in canEvaluateShuffled

Bjorn Pettersson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 16 08:55:07 PDT 2019


bjope added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:1069
+    case Instruction::SRem:
+      // Avoid introducing integer div/rem by undef to avoid undefined behavior.
+      for (int i = 0, e = Mask.size(); i != e; ++i)
----------------
spatel wrote:
> lebedev.ri wrote:
> > Does this apply to FP too? (i'm not seeing that in langref)
> > If it does, then the comment needs updating.
> FDiv (or any other non-constrained FP opcode) is safe; it should not be included in this patch. Might want to add a negative test case to confirm that and make sure nobody makes that mistake going forward.
Oopsy-daisy, good thing there are reviewers. FDiv was actually included by mistake here.

I will duplicate the "test2" test case for udiv/sdiv/urem/fdiv/frem, making the tests for fdiv/frem "negative" (expecting that the transform is done)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69038





More information about the llvm-commits mailing list