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

Reassociate drops NSW/NUW flags #13357

Open
atrick opened this issue May 29, 2012 · 4 comments
Open

Reassociate drops NSW/NUW flags #13357

atrick opened this issue May 29, 2012 · 4 comments
Labels
bugzilla Issues migrated from bugzilla code-quality confirmed Verified by a second party llvm:transforms

Comments

@atrick
Copy link
Contributor

atrick commented May 29, 2012

Bugzilla Link 12985
Version trunk
OS All
Attachments rtest.ll, rtest2.ll

Extended Description

The Reassociate pass unnecessarily drops NSW/NUW flags during canonicalization. If we really want to drop the flags, we should probably do it after indvars where we eliminate sign/zero extension.

I noticed three situations where Reassociation drops flags in a single test case:

  1. shl nsw x, c => mul x, 2**c
  2. sub nsw x, y => add x, (sub 0, y)
  3. add nsw (add nsw x, c1), c2 ; where c1 and c2 have opposite signs.

Attached unit tests:
rtest.ll
rtest2.ll

rtest.ll does some legitimate reassociation in order to fold constants (exhibits case #​3).

rtest2.ll has an additional use of %sub, so doesn't do any actual reassociation or folding, and is closer to the real test case, but it still drops flags everywhere.

rtest2.ll would probably be easier to fix (no case #​3), but it would be nice to handle rtest.ll too. It would be a nice property to preserve the flags if possible regardless of the number of uses

@lattner
Copy link
Collaborator

lattner commented May 30, 2012

This should be straight-forward to fix.

@llvmbot
Copy link
Member

llvmbot commented Nov 8, 2014

  1. shl nsw x, c => mul x, 2**c

Mostly addressed in rr21555.

@llvmbot
Copy link
Member

llvmbot commented Nov 8, 2014

  1. shl nsw x, c => mul x, 2**c

Mostly addressed in rr2155

Errr.. r221555.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 3, 2021
@llvmbot llvmbot added the confirmed Verified by a second party label Jan 26, 2022
@arsenm
Copy link
Contributor

arsenm commented Aug 29, 2023

new-tests.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla code-quality confirmed Verified by a second party llvm:transforms
Projects
None yet
Development

No branches or pull requests

5 participants