[llvm-commits] [llvm] r122704 - in /llvm/trunk: lib/Transforms/Scalar/LoopIdiomRecognize.cpp test/Transforms/LoopIdiom/basic.ll

[Duncan Sands]

Hi Chris,

...
>> In both functions the memset could also take care of the store to the first
>> element, rather than starting from the second element.  However maybe merging
>> the store and the memset should be a job for a different pass.
>
> It's likely that this should be done by memcpy opt, which has logic for merging multiple consecutive stores into a memset.

OK.

>> Secondly, notice that the size of the memset in the second function is -4.
>> Hopefully this will work correctly (i.e. memset 2^32-4 values)!  It did make
>> me wonder what happens if the loop stores say 2^32 or 2^48 values.  Presumably
>> the type of the memset size argument is automagically set to a size that can
>> hold the loop trip count...
>
> I don't understand what you're saying here... is there a miscompilation, or was the original code doing this large store?  -4 is "a big 32-bit number" and is zero extended to i64 on 64-bit targets.

No miscompilation, 2^32-4 is correct.  I took a look at the loopidiom code and
it gets the type of the memset size operand from the SCEV type, so it should
always be big enough, i.e. there is no problem here, sorry to have bothered you
about it.

Ciao, Duncan.