[PATCH] D21246: [InstCombine] Add rule to fold away trunc of partial load
Anna Thomas via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 17 08:48:07 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
> 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`?
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:588
@@ +587,3 @@
+ if (Value *AvailableVal = FindAvailableLoadedValue(
+ LI, LI->getParent(), BBI, DefMaxInstsToScan,
+ /* AA */ nullptr, /* AATags */ nullptr, DestTy))
> Here if we have:
> store i64 %x to %ptr
> %val = load i64* %ptr
> %val.tr = trunc i64 %val to i32
> won't we have a type mismatch and try to replace `%val.tr` with `%x`?
We won't replace `%val.tr` with `%x` since the `DestTy` is `i32`. We do the check in `FindAvailableLoadedValue (LI: %val,.., DestTy: i32)` that `i64` is `BitorNoopPointerCastable` to the PartialType `i32`.
I've added this test case as well now.
More information about the llvm-commits