[PATCH] D21246: [InstCombine] Add rule to fold away trunc of partial load
Anna Thomas via llvm-commits
llvm-commits at lists.llvm.org
Sat Jun 18 13:09:28 PDT 2016
anna added inline comments.
Comment at: lib/Analysis/Loads.cpp:385
@@ +384,3 @@
+ // If we support partial value forwarding to the load (PartialType is
+ // non-null): LI is from a part of the bits (known from PartialType)
+ // used by Load, return the value for those bits. The PartialType is from
> anna wrote:
> > sanjoy wrote:
> > > It feels odd that we have to special-case `PartialType` this way. I think a better design would be to introduce a new function `FindAvailableMemoryContents(Value *Ptr, Type *AccessTy ... < no PartialTy>)` (perhaps with a different name), and have this version of `FindAvailableLoadedValue` dispatch to `FindAvailableMemoryContents` one with `LI->getPointerOperand(), LI->getType()`. `visitTrunc` can then call `FindAvailableMemoryContents` directly.
> > It seems ok, but are we fine with changing `FindAvailableLoadedValue` to being a wrapper function, with all the actual code in `FindAvailableMemoryContents`?
> I don't see any problems with that (you can put them both in this .cpp file for the time being).
Just noticed there is a similar change that went in for the same function checking.
This change adds a new parameter to the function `isLoadCSE`, with default as `nullptr`.
I'll have to rethink the design for my change. it makes sense to create a separate function which checks for partial memory contents. However, we do not want to keep adding extra parameters to the `FindAvailableLoadedValue` function, since this would mean keeping track of the parameters (or atleast knowing what each parameter does) when we call `FindAvailableLoadedValue` from `FindAvailableMemoryContents`.
More information about the llvm-commits