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

InstCombine: incorrect select operand simplification with undef #47040

Closed
nunoplopes opened this issue Sep 30, 2020 · 3 comments
Closed

InstCombine: incorrect select operand simplification with undef #47040

nunoplopes opened this issue Sep 30, 2020 · 3 comments
Labels
bugzilla Issues migrated from bugzilla miscompilation

Comments

@nunoplopes
Copy link
Member

Bugzilla Link 47696
Resolution FIXED
Resolved on Oct 23, 2020 18:33
Version trunk
OS All
Blocks #47292
CC @RKSimon,@nikic,@regehr,@rotateright
Fixed by commit(s) 9d1c8c0

Extended Description

Test: Transforms/InstCombine/select-binop-cmp.ll

Seems to be a regression from https://reviews.llvm.org/D87480
The undef operand in the icmp needs to be fixed to 0 for the transformation to be correct. See here: https://alive2.llvm.org/ce/z/YYhL33

define <2 x i8> @​select_xor_icmp_vec_undef(<2 x i8> %x, <2 x i8> %y, <2 x i8> %z) {
%A = icmp eq <2 x i8> %x, { 0, undef }
%B = xor <2 x i8> %x, %z
%C = select <2 x i1> %A, <2 x i8> %B, <2 x i8> %y
ret <2 x i8> %C
}
=>
define <2 x i8> @​select_xor_icmp_vec_undef(<2 x i8> %x, <2 x i8> %y, <2 x i8> %z) {
%A = icmp eq <2 x i8> %x, { 0, undef }
%C = select <2 x i1> %A, <2 x i8> %z, <2 x i8> %y
ret <2 x i8> %C
}
Transformation doesn't verify!
ERROR: Target's return value is more undefined

Example:
<2 x i8> %x = < poison, #x01 (1) >
<2 x i8> %y = < poison, #x00 (0) >
<2 x i8> %z = < poison, #x01 (1) >

Source:
<2 x i1> %A = < poison, any >
<2 x i8> %B = < poison, #x00 (0) >
<2 x i8> %C = < poison, #x00 (0) >

Target:
<2 x i1> %A = < poison, #x1 (1) >
<2 x i8> %C = < poison, #x01 (1) >
Source value: < poison, #x00 (0) >
Target value: < poison, #x01 (1) >

https://web.ist.utl.pt/nuno.lopes/alive2/index.php?hash=1546033e6d866cfc&test=Transforms%2FInstCombine%2Fselect-binop-cmp.ll

@nikic
Copy link
Contributor

nikic commented Sep 30, 2020

Probably should block this transform if the icmp operand is not guaranteed-not-undef.

@nikic
Copy link
Contributor

nikic commented Oct 1, 2020

Should be fixed by 9d1c8c0.

@aqjune
Copy link
Contributor

aqjune commented Nov 27, 2021

mentioned in issue #47292

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

No branches or pull requests

3 participants