Skip to content

[X86] Premature replacement of TRUNCATE with PACKSS/PACKUS patterns are preventing generic combines #63710

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.

Sign up for GitHub

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

Jump to bottom
Closed
RKSimon opened this issue Jul 6, 2023 · 1 comment
Closed

[X86] Premature replacement of TRUNCATE with PACKSS/PACKUS patterns are preventing generic combines #63710

RKSimon opened this issue Jul 6, 2023 · 1 comment
Assignees
@RKSimon
Labels
backend:X86

Comments

@RKSimon
Copy link
Collaborator

RKSimon commented Jul 6, 2023

The combineVectorTruncation / combineVectorSignBitsTruncation combines are converting generic truncate nodes to target specific packss/packus nodes very early, mainly as a way to retain these patterns before type legalization. But this prevents the use of generic combines that use truncate.

combineVectorTruncation / combineVectorSignBitsTruncation need to be removed and we need better lowering of truncate nodes from illegal result/operand types.

This is very obvious with https://reviews.llvm.org/D152928
@RKSimon RKSimon self-assigned this Jul 6, 2023
@llvmbot
Copy link
Member

llvmbot commented Jul 6, 2023

@llvm/issue-subscribers-backend-x86

RKSimon added a commit that referenced this issue Jul 11, 2023
@RKSimon
[X86] LowerTRUNCATE - improve handling during type legalization to PA…
842a672 
…CKSS/PACKUS patterns

Extend coverage for lowering wide vector types during type legalization to allow us to use PACKSS/PACKUS patterns instead of dropping down to shuffle lowering.

First step towards avoiding premature folds of TRUNCATE to PACKSS/PACKUS nodes as described on Issue #63710 - which causes a large number of regressions on D152928 - we will next need to tweak the TRUNCATE widening in ReplaceNodeResults

Differential Revision: https://reviews.llvm.org/D154592
RKSimon added a commit that referenced this issue Jul 13, 2023
@RKSimon
[X86] Remove combineVectorTruncation and delay general vector trunc t…
451af63 
…o lowering

Stop folding vector truncations to PACKSS/PACKUS patterns prematurely - another step towards Issue #63710. We still prematurely fold to PACKSS/PACKUS if there are sufficient signbits, that will be addressed in a later patch when we remove combineVectorSignBitsTruncation.

This required ReplaceNodeResults to extend handling of sub-128-bit results to SSSE3 (or later) cases, which has allowed us to improve vXi32->vXi16 truncations to use PSHUFB.

I also tweaked LowerTruncateVecPack to recognise widened truncation source operands so the upper elements remain UNDEF (otherwise truncateVectorWithPACK* will constant fold them to allzeros/allones values).
@RKSimon RKSimon mentioned this issue Jul 19, 2023
Closed
RKSimon added a commit that referenced this issue Jul 21, 2023
@RKSimon
[X86] combineBitcastvxi1 - don't prematurely create PACKSS nodes.
65c9153 
Similar to Issue #63710 - by truncating the v8i16 result with a PACKSS node before type legalization, we fail to make use of various folds that rely on TRUNCATE nodes.

This required tweaks to LowerTruncateVecPackWithSignBits to recognise when the truncation source has been widened and to more closely match combineVectorSignBitsTruncation wrt truncating with PACKSS/PACKUS on AVX512 targets.

One of the last stages before we can finally get rid of combineVectorSignBitsTruncation.
RKSimon added a commit that referenced this issue Aug 4, 2023
@RKSimon
[X86] ReplaceNodeResults - widen sub-128-bit vector truncations if it…
e229086 
… would allow them to use PACKSS/PACKUS

We currently just scalarize sub-128-bit vector truncations, but if the input vector has sufficient signbits/zerobits then we should try to use PACKSS/PACKUS with a widened vector with don't care upper elements. Shuffle lowering will struggle to detect this if we wait until the scalarization has been revectorized as a shuffle.

Another step towards issue #63710
RKSimon added a commit that referenced this issue Aug 11, 2023
@RKSimon
[X86] combineConcatVectorOps - extend PACKSS/PACKUS handling to 512-b…
6c119cf 
…it nodes on BWI targets.

Fixes another TRUNCATE -> PACKSS/PACKUS regression when #63710 finally gets fixed
razmser pushed a commit to SuduIDE/llvm-project that referenced this issue Oct 2, 2023
@RKSimon @razmser
[X86] Remove combineVectorSignBitsTruncation and leave TRUNCATE -> PA…
21b3932 
…CKSS/PACKUS to legalization/lowering

Don't prematurely fold TRUNCATE nodes to PACKSS/PACKUS target nodes - we miss out on generic TRUNCATE folds.

Helps some regressions from D152928 and llvm#63946

Fixes llvm#63710
razmser pushed a commit to SuduIDE/llvm-project that referenced this issue Oct 2, 2023
@RKSimon @razmser
[X86] Remove combineVectorSignBitsTruncation and leave TRUNCATE -> PA…
da9153c 
…CKSS/PACKUS to legalization/lowering

Don't prematurely fold TRUNCATE nodes to PACKSS/PACKUS target nodes - we miss out on generic TRUNCATE folds.

Helps some regressions from D152928 and llvm#63946

Fixes llvm#63710
razmser pushed a commit to SuduIDE/llvm-project that referenced this issue Oct 2, 2023
@RKSimon @razmser
[X86] Remove combineVectorSignBitsTruncation and leave TRUNCATE -> PA…
afbe72a 
…CKSS/PACKUS to legalization/lowering

Don't prematurely fold TRUNCATE nodes to PACKSS/PACKUS target nodes - we miss out on generic TRUNCATE folds.

Helps some regressions from D152928 and llvm#63946

Fixes llvm#63710
razmser pushed a commit to SuduIDE/llvm-project that referenced this issue Oct 3, 2023
@RKSimon @razmser
[X86] Remove combineVectorSignBitsTruncation and leave TRUNCATE -> PA…
f29359c 
…CKSS/PACKUS to legalization/lowering

Don't prematurely fold TRUNCATE nodes to PACKSS/PACKUS target nodes - we miss out on generic TRUNCATE folds.

Helps some regressions from D152928 and llvm#63946

Fixes llvm#63710
razmser pushed a commit to SuduIDE/llvm-project that referenced this issue Oct 3, 2023
@RKSimon @razmser
[X86] Remove combineVectorSignBitsTruncation and leave TRUNCATE -> PA…
0db7989 
…CKSS/PACKUS to legalization/lowering

Don't prematurely fold TRUNCATE nodes to PACKSS/PACKUS target nodes - we miss out on generic TRUNCATE folds.

Helps some regressions from D152928 and llvm#63946

Fixes llvm#63710
razmser pushed a commit to SuduIDE/llvm-project that referenced this issue Oct 6, 2023
@RKSimon @razmser
[X86] Remove combineVectorSignBitsTruncation and leave TRUNCATE -> PA…
54c5e16 
…CKSS/PACKUS to legalization/lowering

Don't prematurely fold TRUNCATE nodes to PACKSS/PACKUS target nodes - we miss out on generic TRUNCATE folds.

Helps some regressions from D152928 and llvm#63946

Fixes llvm#63710
razmser pushed a commit to SuduIDE/llvm-project that referenced this issue Oct 11, 2023
@RKSimon @razmser
[X86] Remove combineVectorSignBitsTruncation and leave TRUNCATE -> PA…
bd62b65 
…CKSS/PACKUS to legalization/lowering

Don't prematurely fold TRUNCATE nodes to PACKSS/PACKUS target nodes - we miss out on generic TRUNCATE folds.

Helps some regressions from D152928 and llvm#63946

Fixes llvm#63710
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@RKSimon RKSimon

Labels
backend:X86
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@RKSimon @llvmbot