[PATCH] D108734: [InstCombine] Replace icmp invariant group operands with the invariant group's operands

[Piotr Padlewski via Phabricator via llvm-commits]

Prazek accepted this revision.
Prazek added a comment.
This revision is now accepted and ready to land.

LGTM! Great work with looking for regressions like that.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:5715
+      match(I.getOperand(1), m_Zero())) {
+    if (Op->isLaunderOrStripInvariantGroup())
+      return ICmpInst::Create(Instruction::ICmp, I.getPredicate(),
----------------
I wonder if the fact that usually there are bitcasts around strip/launder could be problematic.
I think this does not handle case like:


  %b1 = bitcast i32* %p to i8*
  %b2 = strip(%b1)
  %b3 = bitcast i8* %b2 to i32*

  icmp %b3, null

But it might be not a problem, if this code gets cannonicalized to:

  %b1 = bitcast i32* %p to i8*
  %b2 = strip(%b1)
  
  icmp %b2, null


Could you double check if this would be optimized by InstCombine allone, or that there is any pass that would cannonicalize it?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108734