This is an archive of the discontinued LLVM Phabricator instance.

[WIP][X86] Move shl(x,1) -> add(x,x) vector fold to ISEL (PR50468)
AbandonedPublic

Authored by RKSimon on Jul 23 2021, 9:18 AM.

Details

Summary

Partial alternative to D106675 to avoid the undef issues identified in PR50468 by using alternative isel patterns for vXi16/vXi32/vXi64 shl-by-one.

This affects many cases which weren't being picked up in time by combineShiftLeft() - which since this is a performance combine is probably a good thing - plus it avoids some HasOneUse issues that allows some additional optimization.

The vXi8 shl-by-one would still need to be performed using FREEZE - I don't know if anyone has concerns about having different methods?

Diff Detail

Event Timeline

RKSimon created this revision.Jul 23 2021, 9:18 AM
RKSimon requested review of this revision.Jul 23 2021, 9:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 23 2021, 9:18 AM
craig.topper added inline comments.Jul 23 2021, 9:31 AM
llvm/lib/Target/X86/X86InstrAVX512.td
5925

Do we really need AddedComplexity? Especially as high as 400. The explicit immediate should automatically rank higher than any immediate.

craig.topper added inline comments.Jul 23 2021, 9:34 AM
llvm/test/CodeGen/X86/stack-folding-int-avx512.ll
6315–6316

This is a bit sad. Would it make sense to convert add back to shift when folding a load?

RKSimon updated this revision to Diff 361434.Jul 24 2021, 4:39 AM

Remove unnecessary AddedComplexity

RKSimon marked an inline comment as done.Jul 24 2021, 4:43 AM
RKSimon added inline comments.
llvm/test/CodeGen/X86/stack-folding-int-avx512.ll
6315–6316

It would, although I'm struggling to think of a way to do that - foldMemoryOperandImpl won't ever get the chance.

Another option is we just remove the ISEL patterns from AVX512 targets entirely.

RKSimon planned changes to this revision.Sep 28 2021, 12:55 AM
RKSimon abandoned this revision.Jul 30 2022, 5:28 AM

Abandoning this - I'm going to go with the FREEZE approach (D106675) once the necessary support has been added to DAG (D130646 etc.).

Herald added a project: Restricted Project. · View Herald TranscriptJul 30 2022, 5:28 AM
Herald added a subscriber: StephenFan. · View Herald Transcript