[PATCH] D108734: [InstCombine] Remove invariant group intrinsincs when comparing against null

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks marked an inline comment as done.
aeubanks added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:5709-5711
+          I.getOperand(0)->getType()->getPointerAddressSpace())) {
+    return nullptr;
+  }
----------------
lebedev.ri wrote:
> 
it's hard to read with a multi-line if condition IMO


================
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(),
----------------
Prazek wrote:
> 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?
> 
added a test to verify that instcombine takes care of this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108734