This is an archive of the discontinued LLVM Phabricator instance.

[X86][WIP] Enable `foldSelectWithIdentityConstant` for scalar types.
Needs ReviewPublic

Authored by goldstein.w.n on Apr 25 2023, 2:28 PM.

Details

Summary

There are a variety of regressions at the moment, but it would be nice
if we could turn this on in the backend effectively so that
InstCombine can generically fold (select c, BinOp(X, Y), X) ->
BinOp(X, (select c, Y, Indentity)) which is generally preferable in
the middle-end.

Diff Detail

Event Timeline

goldstein.w.n created this revision.Apr 25 2023, 2:28 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 25 2023, 2:28 PM
goldstein.w.n requested review of this revision.Apr 25 2023, 2:28 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 25 2023, 2:28 PM
pengfei added inline comments.Apr 25 2023, 8:40 PM
llvm/test/CodeGen/X86/avx512-intrinsics-fast-isel.ll
1889–1893

Is this change intentional?

llvm/test/CodeGen/X86/bool-simplify.ll
56–60

This looks regression to me. ditto below.

llvm/test/CodeGen/X86/bool-vector.ll
68

Introducing many branches, which doesn't look good.

llvm/test/CodeGen/X86/fold-select.ll
10–16

Regression? ditto below.

goldstein.w.n added inline comments.Apr 25 2023, 9:20 PM
llvm/test/CodeGen/X86/fold-select.ll
10–16

Yeah mostly regressions. Working on trying to clean it up.
Have a bit of promise restricting to only if both arms are constant but still many.

RKSimon added inline comments.Apr 27 2023, 6:26 AM
llvm/test/CodeGen/X86/bool-vector.ll
5

Add X86-SSE2/X64-SSE2 to recover lost coverage

7

Add X86-AVX2/X64-AVX2 to recover lost coverage

RKSimon added inline comments.May 18 2023, 10:03 AM
llvm/test/tools/llvm-locstats/locstats.ll
1 ↗(On Diff #523424)

Is this necessary?

goldstein.w.n marked an inline comment as done.May 18 2023, 10:19 AM

Fix bad test change