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

CVP: adding nuw flags not correct in the presence of undef #41963

Closed
nunoplopes opened this issue Jul 14, 2019 · 1 comment
Closed

CVP: adding nuw flags not correct in the presence of undef #41963

nunoplopes opened this issue Jul 14, 2019 · 1 comment
Labels
bugzilla Issues migrated from bugzilla

Comments

@nunoplopes
Copy link
Member

Bugzilla Link 42618
Resolution FIXED
Resolved on Mar 31, 2020 07:46
Version trunk
OS All
CC @jdoerfert,@luqmana,@nikic,@regehr

Extended Description

The following unit test shows incorrect behavior regarding undef inputs:
$ opt -correlated-propagation -cvp-dont-add-nowrap-flags=false Transforms/CorrelatedValuePropagation/sub.ll

The issue is the branch on undef, which if we assume is a non-deterministic jump, leads to branch %then, which then does a 'sub nuw' on an undef (which is poison).

Possible solutions: same as in #​42617.

define i32 @​test11(* %p, i32 %i) {
%limit = load i32, * %p, align 4
%range_l#0%limit = icmp sge i32 %limit, 0
%range_h#0%limit = icmp slt i32 %limit, 2147483647
%range#0%limit = and i1 %range_l#0%limit, %range_h#0%limit
assume_non_poison i1 %range#0%limit
%within.1 = icmp slt i32 %limit, %i
%i.minus.7 = add i32 %i, 4294967289
%within.2 = icmp slt i32 %limit, %i.minus.7
%within = and i1 %within.1, %within.2
br i1 %within, label %then, label %else

%then:
%i.minus.6 = sub i32 %i, 6
ret i32 %i.minus.6

%else:
ret i32 0
}
=>
define i32 @​test11(* %p, i32 %i) {
%limit = load i32, * %p, align 4
%range_l#0%limit = icmp sge i32 %limit, 0
%range_h#0%limit = icmp slt i32 %limit, 2147483647
%range#0%limit = and i1 %range_l#0%limit, %range_h#0%limit
assume_non_poison i1 %range#0%limit
%within.1 = icmp slt i32 %limit, %i
%i.minus.7 = add i32 %i, 4294967289
%within.2 = icmp slt i32 %limit, %i.minus.7
%within = and i1 %within.1, %within.2
br i1 %within, label %then, label %else

%then:
%i.minus.6 = sub nsw nuw i32 %i, 6
ret i32 %i.minus.6

%else:
ret i32 0
}
Transformation doesn't verify!
ERROR: Target is more poisonous than source

Example:

  • %p = #x2dc920000000
    i32 %i = undef

Source:
i32 %limit = #x00000000 (0)
i1 %range_l#0%limit = #x1 (1)
i1 %range_h#0%limit = #x1 (1)
i1 %range#0%limit = #x1 (1)
i1 %within.1 = undef
i32 %i.minus.7 = undef
i1 %within.2 = undef
i1 %within = undef
i32 %i.minus.6 = undef

Target:
i32 %limit = #x00000000 (0)
i1 %range_l#0%limit = #x1 (1)
i1 %range_h#0%limit = #x1 (1)
i1 %range#0%limit = #x1 (1)
i1 %within.1 = undef
i32 %i.minus.7 = undef
i1 %within.2 = undef
i1 %within = undef
i32 %i.minus.6 = poison
Source value: undef
Target value: poison

@nunoplopes
Copy link
Member Author

We now have langref stating that jump on undef is UB. So this one is good.

@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
Projects
None yet
Development

No branches or pull requests

1 participant