[PATCH] D28962: Add BFI in constanthoisting pass and do the hoisting selectively

[Wei Mi via Phabricator via llvm-commits]

wmi added inline comments.


================
Comment at: lib/Transforms/Scalar/ConstantHoisting.cpp:678
+      Instruction *Base =
+          new BitCastInst(ConstInfo.BaseConstant, Ty, "const", IP);
+      DEBUG(dbgs() << "Hoist constant (" << *ConstInfo.BaseConstant
----------------
davidxl wrote:
> davidxl wrote:
> > There are three cases:
> > 1) new insertion point introduced
> > 2) the BB originally has a base constant use
> > 3) the BB originally has a constant use but need rebase.
> > 
> > For case 1) and 3), new insertion is needed, but for 2) it can be skipped.
> Correction: 2) can only be skipped when there are no other uses dominated by it -- so it is probably better to not to skip and rewrite the original use
I thought it was not going to change the generated code if we do a rebase for a base constant use. That is why for the code simplicity, I choose to insert materialization and rebase code anyway.  

It will change 

BB:
%inc = add i64 %5, 214748364701

to

BB:
%const = bitcase i64 214748364701 to i64
 ...
%inc = add i64 %5, %const

Because 214748364701 is a very large number (The precondition it is treated as constant hoisting candidate) and it cannot be encoded in add instruction, we need the materialization code %const = ...  anyway in the end. 


Repository:
  rL LLVM

https://reviews.llvm.org/D28962