[PATCH] D25914: Redo store splitting in CodeGenPrepare
Wei Mi via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 28 14:05:29 PST 2016
wmi added inline comments.
================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:5300-5301
+ const TargetLowering &TLI) {
+ unsigned HalfValBitSize =
+ DL.getTypeSizeInBits(SI.getValueOperand()->getType()) / 2;
+
----------------
majnemer wrote:
> wmi wrote:
> > majnemer wrote:
> > > This looks wrong. Shouldn't it be `getTypeStoreSizeInBits` instead of `getTypeSizeInBits`?
> > They are the same here because the type of the store value must have power of 2 size if it is a merged store. But you remind me to add some testcases: @int31_float_pair, @int15_float_pair, @int7_float_pair added in the testcase.
> But aren't i1, i2 and i4 powers of two?
>
> If we stored an i1, `HalfValBitSize` would be zero which sounds problematic.
The store val will at least be i2 because it is a merged val from two smaller vals.
If the store val is an i2 which are combined from an {i1, i1} pair, and we use getTypeStoreSizeInBits to compute the HalfValBitSize, the HalfValBitSize will be 4. It means the split store size will be 4 bits. It is not what we expect. We expect the type of split store is i1.
I cannot add add an i1_i1_pair test because now the target query will return false for int pair. But I have verified that the i1_i1_pair test worked correctly to use getTypeSizeInBits by disabling the target query temporarily.
Repository:
rL LLVM
https://reviews.llvm.org/D25914
More information about the llvm-commits
mailing list