[PATCH] D37832: Eliminate PHI (int typed) which is only used by inttoptr

Wei Mi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 27 10:12:48 PDT 2017


wmi added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombinePHI.cpp:72
+//    int_ptr = BitCast(ptr_ptr)
+//    int_init = Load(int_ptr)
+//    br label %bb2
----------------
hfinkel wrote:
> davidxl wrote:
> > davidxl wrote:
> > > hfinkel wrote:
> > > > I don't understand why you're looking for a load here specifically. It's a good example, because it shows that you might need to look for something here other than a ptrtoint, but that seems to motivate taking a general input (not specifically looking for this load as you do below).
> > > Checking for loads because instcombine can fold the inserted inttoptr away. Same for PHI defs. For other instructions, we are basically just hoisting inttoptr above to the PHI operand, which is unclear it is a win.
> > Also general input is already handled first -- by looking in forward direction if there is an existing inttoptr that can be reused.  
> > 
> > bb1:
> > %arg_int64 = <general operation>
> > ...
> > %ptr_val = inttoptr i64 %arg_int64 to float*
> > 
> > ...
> > bbm:
> >   %int64_phi = PHI ([%arg_int64, %bb1], [...])
> >    %ptr_val = inttoptr i64 %int64_phi to float*
> > 
> > ==>
> > 
> >    %ptr_val = PHI([%ptr_val, %bb1], [...])
> >   
> > 
> > 
> > Checking for loads because instcombine can fold the inserted inttoptr away. Same for PHI defs. For other instructions, we are basically just hoisting inttoptr above to the PHI operand, which is unclear it is a win.
> 
> Seems like a win to me. It will make AA more powerful in the loop. Besides, we can have a separate combine for
>   inttoptr(load(bitcast(pp))) => load(pp)
> 
> (assuming we can prove what's necessary to make AA on these pointers safe)
It seems like a win to me too. By hoisting inttoptr above, no inttoptr/ptrtoint is inside of the loop blocking phi nodes from being recognized as induction variables so loop vectorizer/indvar simplify/lsr will benefit from it.  


https://reviews.llvm.org/D37832





More information about the llvm-commits mailing list