[LLVMdev] [ScalarEvolution] code on nsw inconsistent with comment

[Sanjoy Das]

The code looks incorrect, AFAICT.  Can you write a test case where
this leads to a miscompile, just to confirm?  I suspect this will show
up in something like

for (...) {
  if (cond)
    t = inbounds_gep(A, idx)
  m = gep(A, idx)
  v = m s< A
}

SCEV should incorrectly fold v to false.

On Mon, May 11, 2015 at 1:02 PM, Jingyue Wu <jingyue at google.com> wrote:
> +llvmdev
>
> On Mon, May 11, 2015 at 11:44 AM, Jingyue Wu <jingyue at google.com> wrote:
>>
>> Hi Andrew and Sanjoy,
>>
>> I was looking at ScalarEvolution::createNodeForGEP
>> (http://llvm.org/docs/doxygen/html/ScalarEvolution_8cpp_source.html#l03702),
>> and noticed that the code that applies nsw flags to the resultant SCEV is
>> inconsistent with the comment.
>>
>> While the comment says
>>
>> 03709   // Don't blindly transfer the inbounds flag from the GEP
>> instruction to the
>> 03710   // Add expression, because the Instruction may be guarded by
>> control flow
>> 03711   // and the no-overflow bits may not be valid for the expression in
>> any
>> 03712   // context.
>> 03713   SCEV::NoWrapFlags Wrap = GEP->isInBounds() ? SCEV::FlagNSW :
>> SCEV::FlagAnyWrap;
>>
>> the code still transfers nsw to LocalOffset and BaseS+TotalOffset. Should
>> the code then be fixed?
>>
>> Jingyue
>
>