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

[VectorCombine] miscompilation #114901

Closed
bongjunj opened this issue Nov 5, 2024 · 1 comment
Closed

[VectorCombine] miscompilation #114901

bongjunj opened this issue Nov 5, 2024 · 1 comment

Comments

@bongjunj
Copy link

bongjunj commented Nov 5, 2024

Alive2 report: https://alive2.llvm.org/ce/z/YwSTI4

----------------------------------------
define i1 @icmp_xor_v4i32.2(<4 x i32> %a) {
#0:
  %e1 = extractelement <4 x i32> %a, i32 3
  %e2 = extractelement <4 x i32> %a, i32 1
  %cmp1 = icmp sgt i32 %e1, 42
  %cmp2 = icmp sgt i32 %e2, 4294967288
  %#1 = ashr i1 %cmp1, %cmp2
  ret i1 %#1
}
=>
define i1 @icmp_xor_v4i32.2(<4 x i32> %a) {
#0:
  %#1 = icmp sgt <4 x i32> %a, { poison, 4294967288, poison, 42 }
  %shift = shufflevector <4 x i1> %#1, <4 x i1> poison, 4294967295, 3, 4294967295, 4294967295
  %#2 = ashr <4 x i1> %#1, %shift
  %#3 = extractelement <4 x i1> %#2, i64 1
  ret i1 %#3
}
Transformation doesn't verify!

ERROR: Target is more poisonous than source

Example:
<4 x i32> %a = < #x00000000 (0), #xfffffff8 (4294967288, -8), #x00000000 (0), #x0000002b (43) >

Source:
i32 %e1 = #x0000002b (43)
i32 %e2 = #xfffffff8 (4294967288, -8)
i1 %cmp1 = #x1 (1)
i1 %cmp2 = #x0 (0)
i1 %#1 = #x1 (1)

Target:
<4 x i1> %#1 = < poison, #x0 (0), poison, #x1 (1) >
<4 x i1> %shift = < poison, #x1 (1), poison, poison >
<4 x i1> %#2 = < poison, poison, poison, poison >
i1 %#3 = poison
Source value: #x1 (1)
Target value: poison

Summary:
  0 correct transformations
  1 incorrect transformations
  0 failed-to-prove transformations
  0 Alive2 errors
@RKSimon
Copy link
Collaborator

RKSimon commented Nov 5, 2024

It looks like the ashr operands have been commuted in the fold - will take a look later today

@RKSimon RKSimon closed this as completed in 05e838f Nov 5, 2024
PhilippRados pushed a commit to PhilippRados/llvm-project that referenced this issue Nov 6, 2024
PhilippRados pushed a commit to PhilippRados/llvm-project that referenced this issue Nov 6, 2024
…inops

The fold needs to be adjusted to correctly track the LHS/RHS operands, which will take some refactoring, for now just disable the fold in this case.

Fixes llvm#114901
RKSimon added a commit that referenced this issue Nov 6, 2024
…ve binops

#114901 exposed that foldExtractedCmps didn't account for non-commutative binops, and were disabled by 05e838f

This patch re-enables support for non-commutative binops by ensuring that the LHS/RHS arg order of the binop is retained.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants