This patch updates the remaining transformations to create insertelement/shufflevector with poison placeholder.
Details
Details
Diff Detail
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Unit Tests
Event Timeline
| llvm/lib/CodeGen/CodeGenPrepare.cpp | ||
|---|---|---|
| 6726–6727 ↗ | (On Diff #313736) | Similar to my comment in D93793 - if we convert this (and any others like this) into the unary CreateShuffleVector call, we can reduce the number of code diffs and chances that something is escaping the transition from undef to poison. |
Comment Actions
Alive2 result: Running Alive2 finds one failure, which is fixed by updating the shufflevector semantics to return poison on undef/poison mask (D93818):
Transforms/InstCombine/shufflevector-div-rem-inseltpoison.ll
----------------------------------------
define <2 x float> @test_fdiv(float %a, float %b, i1 %cmp) {
%0:
%splatinsert = insertelement <2 x float> poison, float %a, i32 0
%denom = insertelement <2 x float> { 3.000000, poison }, float 1.000000, i32 1
%t1 = fdiv <2 x float> %splatinsert, %denom
%splat.op = shufflevector <2 x float> %t1, <2 x float> poison, 4294967295, 0 ; THIS BLOCKS POISON
%t2 = select i1 %cmp, <2 x float> { 77.000000, 99.000000 }, <2 x float> %splat.op
ret <2 x float> %t2
}
=>
define <2 x float> @test_fdiv(float %a, float %b, i1 %cmp) {
%0:
%1 = insertelement <2 x float> poison, float %a, i32 1
%2 = fdiv <2 x float> %1, { undef, 3.000000 }
%t2 = select i1 %cmp, <2 x float> { 77.000000, 99.000000 }, <2 x float> %2
ret <2 x float> %t2
}
Transformation doesn't verify!
ERROR: Target is more poisonous than sourceDiffs in llvm/test and clang/test: I checked that all of these are replacements of undef with poison, and there was no larger change.
Remaining undef placeholder: After this patch, there are two places where undef still shows up as a placeholder from llvm tests: I couldn't find where they are coming from. :(
If someone points me where they're likely to appear, I'll fix these.
$ ag "shufflevector <.*> .*, <.*> undef" | grep inseltpoison Transforms/InstCombine/X86/x86-pshufb-inseltpoison.ll $ ag "insertelement <.*> undef" | grep inseltpoison Transforms/InstCombine/AMDGPU/amdgcn-demanded-vector-elts-inseltpoison.ll
Comment Actions
I'll split this patch into smaller pieces, so they're able to get reviewed more easily.
Comment Actions
Factored out non-instcombine diffs; I'll split the instcombine diffs into several patches too
| llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp | ||
|---|---|---|
| 1221 | This change is okay because it is splat (only the first vector of shufflevector was accessed) | |
| llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp | ||
| 551 | ||
| 602 | Okay to be poison (the added comment above replaceExtractElements has details) | |
| 1132 | Since this is creating a splat, this shufflevector change and the insertelement change above is okay. | |
| 1344 | Values is the RHS of a new shufflevector below. | |
| 1441 | When LR.second was nullptr, it was simply a placeholder (so okay to be poison) | |
| 2355 | As seen in line 2353, Elts' indices are smaller than LHSWidth, so okay to be poison. | |
clang-format not found in user's PATH; not linting file.