LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 27707 - Wrong unsigned rem operation when compared to zero
Summary: Wrong unsigned rem operation when compared to zero
Status: RESOLVED FIXED
Alias: None
Product: Polly
Classification: Unclassified
Component: Optimizer (show other bugs)
Version: unspecified
Hardware: PC Windows NT
: P normal
Assignee: Polly Development Mailinglist
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-05-10 19:51 PDT by Chris Jenneisch
Modified: 2016-06-10 11:46 PDT (History)
5 users (show)

See Also:
Fixed By Commit(s):


Attachments
test (1.78 KB, application/octet-stream)
2016-05-10 19:51 PDT, Chris Jenneisch
Details
test (2.74 KB, application/octet-stream)
2016-06-09 20:47 PDT, Huihui Zhang
Details
test (1.78 KB, application/octet-stream)
2016-06-09 20:51 PDT, Huihui Zhang
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Jenneisch 2016-05-10 19:51:48 PDT
Created attachment 16356 [details]
test

Attached test is a simple pointer iteration.

%struct.A = type { i32, i64, i8 }

; Function Attrs: norecurse nounwind
define void @foo1(%struct.A* %a1, %struct.A* readnone %b1) #0 {
entry:
  br label %entry.split

entry.split:                                      ; preds = %entry
  %cmp4 = icmp eq %struct.A* %a1, %b1
  br i1 %cmp4, label %for.cond.cleanup, label %for.body.preheader

for.body.preheader:                               ; preds = %entry.split
  br label %for.body

for.cond.cleanup.loopexit:                        ; preds = %for.body
  br label %for.cond.cleanup

for.cond.cleanup:                                 ; preds = %for.cond.cleanup.loopexit, %entry.split
  ret void

for.body:                                         ; preds = %for.body.preheader, %for.body
  %start.05 = phi %struct.A* [ %incdec.ptr, %for.body ], [ %a1, %for.body.preheader ]
  %a = getelementptr inbounds %struct.A, %struct.A* %start.05, i64 0, i32 0
  %0 = load i32, i32* %a, align 8
  %add = add nsw i32 %0, 1
  store i32 %add, i32* %a, align 8
  %incdec.ptr = getelementptr inbounds %struct.A, %struct.A* %start.05, i64 1
  %cmp = icmp eq %struct.A* %incdec.ptr, %b1
  br i1 %cmp, label %for.cond.cleanup.loopexit, label %for.body
}

Now consider the polly condition generated in the schedule,

opt  -polly-process-unprofitable -polly-code-generator=isl -polly-scops -
analyze test.ll -S

:: isl ast :: foo1 :: entry.split => for.cond.cleanup

if (1 && 0 == (a1 >= b1 + 1 || a1 >= b1 + 24 * floord(a1 - b1, 24) + 1 || b1 >= a1 + 9223372036854775848))

    if ((a1 - b1) % 24 == 0)
      for (int c0 = 0; c0 < (-a1 + b1) / 24; c0 += 1)
        Stmt_for_body(c0);

else
    {  /* original code */ }


In polly.cond we generate a urem for the (a1 - b1) % 24,

opt  -polly-process-unprofitable -polly-code-generator=isl -polly-scops  -polly-codegen  test.ll -S


polly.cond:                                       ; preds = %polly.start
  %23 = ptrtoint %struct.A* %a1 to i64
  %24 = ptrtoint %struct.A* %b1 to i64
  %25 = sub nsw i64 %23, %24
  %pexp.zdiv_r = urem i64 %25, 24   <==========
  %26 = icmp eq i64 %pexp.zdiv_r, 0
  br i1 %26, label %polly.then, label %polly.else

In case of when (a1 - b1) is negative, an unsigned remainder operation can give unexpected results. For example if (a1 - b1) = -48 (0xFFFFFFFFFFFFFFD0), the modulo with 24 would be 16.
In such cases we need to generate srem.
Comment 1 Chris Jenneisch 2016-05-10 19:53:55 PDT
I created patch http://reviews.llvm.org/D20145 to address this.
Comment 2 Michael Kruse 2016-06-03 13:56:34 PDT
Fixed in r271707. Thanks Chris for finding the bug and its patch.
Comment 3 Huihui Zhang 2016-06-09 20:47:29 PDT
Created attachment 16502 [details]
test
Comment 4 Huihui Zhang 2016-06-09 20:51:22 PDT
Created attachment 16503 [details]
test

Sorry, attached wrong test. Reattaching the origibal
Comment 5 Tobias Grosser 2016-06-10 07:29:34 PDT
Hi Huihui,

good to read you. As this bug is already resolved, I wonder why you posted a changed test case? Is there something we should have a look or is no further feedback needed?

Best,
Tobias
Comment 6 Huihui Zhang 2016-06-10 11:46:16 PDT
Hey Tobi,

Sorry, I accidentally attach the wrong test in this bug. I wanted to attach in bug 28071, so I reattach the original.