Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong unsigned rem operation when compared to zero #28081

Closed
llvmbot opened this issue May 11, 2016 · 6 comments
Closed

Wrong unsigned rem operation when compared to zero #28081

llvmbot opened this issue May 11, 2016 · 6 comments
Labels
bugzilla Issues migrated from bugzilla polly

Comments

@llvmbot
Copy link
Collaborator

llvmbot commented May 11, 2016

Bugzilla Link 27707
Resolution FIXED
Resolved on Jun 10, 2016 11:46
Version unspecified
OS Windows NT
Attachments test
Reporter LLVM Bugzilla Contributor
CC @Meinersbur,@tobiasgrosser

Extended Description

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.

@llvmbot
Copy link
Collaborator Author

llvmbot commented May 11, 2016

I created patch http://reviews.llvm.org/D20145 to address this.

@Meinersbur
Copy link
Member

Fixed in r271707. Thanks Chris for finding the bug and its patch.

@llvmbot
Copy link
Collaborator Author

llvmbot commented Jun 10, 2016

test

@llvmbot
Copy link
Collaborator Author

llvmbot commented Jun 10, 2016

test
Sorry, attached wrong test. Reattaching the origibal

@tobiasgrosser
Copy link
Contributor

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

@llvmbot
Copy link
Collaborator Author

llvmbot commented Jun 10, 2016

Hey Tobi,

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

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla polly
Projects
None yet
Development

No branches or pull requests

3 participants