[prev in list] [next in list] [prev in thread] [next in thread] 

List:       llvm-commits
Subject:    [PATCH] D51396: [Constant Hoisting] Hoisting Constant GEP Expressions
From:       "Z. Zheng via Phabricator via llvm-commits" <llvm-commits () lists ! llvm ! org>
Date:       2018-08-31 23:08:21
Message-ID: 921785afe37d6ca397fd2f04dbc04b0e () localhost ! localdomain
[Download RAW message or body]

zzheng marked 4 inline comments as done.
zzheng added inline comments.


================
Comment at: lib/Transforms/Scalar/ConstantHoisting.cpp:407
+  int Cost = 3;;
+  if (Cost > TTI->getIntImmCost(Instruction::Add, 1, Offset, PtrIntTy)) {
+    ConstCandVecType &ExprCandVec = ConstGEPCandMap[BaseGV];
----------------
efriedma wrote:
> zzheng wrote:
> > efriedma wrote:
> > > This still seems weird; we only hoist if the cost is less than 3?
> > We want to hoist when cost of hoisting is smaller than cost of not-hoisting.
> > 
> > Currently a constant GEP whose base pointer is a GV, is lowered to a CP entry. \
> > The initial value of Cost reflects cost of loading this CP entry. 
> > If loading from CP is always more expensive than computing it by <base + offset>, \
> > perhaps we can get rid of this if condition?
> Making it unconditional is probably okay.
> 
> Well, we might not want to hoist if the offset is so small we aren't actually \
> getting any benefit.  (e.g. loading from `[r0]`  and `[r0, #4]`  isn't really \
> better than loading from `[r0, #8]` and `[r0, #12]`).
This patch tries to change from loading a group of CP entries to loading one CP entry \
and loading the reset by <Base + Offset>.

However, that's the case on our internal tree. We have a patch that lowers constant \
GEPs to CP entries


Repository:
  rL LLVM

https://reviews.llvm.org/D51396



_______________________________________________
llvm-commits mailing list
llvm-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic