[PATCH] D68022: [InstCombine] Don't assume CmpInst has been visited in getFlippedStrictnessPredicateAndConstant

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri accepted this revision.
lebedev.ri added a comment.

Still LG.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:5144-5146
     // TODO? If the edge cases for vectors were guaranteed to be handled as they
     // are for scalar, we could remove the min/max checks. However, to do that,
     // we would have to use insertelement/shufflevector to replace edge values.
----------------
lebedev.ri wrote:
> bjope wrote:
> > lebedev.ri wrote:
> > > Comment needs updating - the scalar case is no longer guaranteed to be handled,
> > > so we can't talk about in the vector case.
> > Maybe I've misunderstood something.
> > 
> > I've assumed that the ConstantIsOk check verified that the edge case had been handled (for the scalar case). And if not we bail out.
> > So in order to reach the code below this if-elseif-else, for scalars, the edge case should have been handled. Isn't that still true?
> > 
> > I can of course remove this TODO if you think that it isn't valid any longer.
> ```
>     // TODO? If the edge cases for vectors were guaranteed to be handled as they
>     // are for scalar
> ```
> implies that we'll never get tautological inputs for scalars.
> The old code asserted for that.
> But now you remove that assert, thus saying there is no such guarantee scalars.
> Therefore similarly we can't possibly ever have a similar guarantee for vectors.
> At least that is how i parse all that after a quick look.
Please mark comments as done as appropriate


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68022