This is an archive of the discontinued LLVM Phabricator instance.

[X86] Prefer fpext(splat(X)) to splat(fpext(x)).
ClosedPublic

Authored by FreddyYe on Jan 12 2023, 10:39 PM.

Details

Summary

This patch is to fix regression of D122875. X86 has fpext instructions
supporting rmb form, which takes advantage of fpext(fplat(X)) than
splat(fpext(X)).

Diff Detail

Event Timeline

FreddyYe created this revision.Jan 12 2023, 10:39 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 12 2023, 10:39 PM
FreddyYe requested review of this revision.Jan 12 2023, 10:39 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 12 2023, 10:39 PM
FreddyYe added a comment.EditedJan 12 2023, 10:39 PM

This patch is to fix regression below: https://gcc.godbolt.org/z/v3nnbKP1j

skan added a comment.Jan 12 2023, 10:43 PM

This patch is to fix regression below: https://reviews.llvm.org/D141657

You are using the link of this revision...

pengfei added inline comments.Jan 13 2023, 1:30 AM
llvm/test/CodeGen/X86/prefer-fpext-splat.ll
1

Pre-commit the test to show the problem?

RKSimon added inline comments.Jan 13 2023, 2:39 AM
llvm/test/CodeGen/X86/prefer-fpext-splat.ll
6

remove the global constant and use a function argument:

define <4 x double> @prefer(float* %p) {
entry:
  %0 = load float, float* %p, align 4
FreddyYe updated this revision to Diff 488946.Jan 13 2023, 4:36 AM
FreddyYe marked an inline comment as done.

Address comments. THX for review!

skan added inline comments.Jan 13 2023, 4:46 AM
llvm/lib/Target/X86/X86ISelLowering.cpp
6017

Why not return Opc != ISD::FP_EXTEND?

FreddyYe updated this revision to Diff 488951.Jan 13 2023, 4:52 AM
FreddyYe marked an inline comment as done.

Address comments. THX for review!

FreddyYe updated this revision to Diff 488963.Jan 13 2023, 5:16 AM
FreddyYe marked an inline comment as done.

Rebase pre-commit test.

skan accepted this revision.Jan 13 2023, 5:20 AM

LGTM

This revision is now accepted and ready to land.Jan 13 2023, 5:20 AM
RKSimon added inline comments.Jan 13 2023, 5:56 AM
llvm/test/CodeGen/X86/prefer-fpext-splat.ll
2–6

Please can you add sse/avx1/avx2 test coverage as well just to ensure non-avx512 targets prefer this as well?

lebedev.ri added inline comments.
llvm/test/CodeGen/X86/prefer-fpext-splat.ll
7–8

While there, please add i8/i16/i32/i64/double/half tests

pengfei added inline comments.Jan 13 2023, 7:33 PM
llvm/test/CodeGen/X86/prefer-fpext-splat.ll
27–54

Change to ptr.

skan added a comment.Jan 13 2023, 8:15 PM

Minor suggestion: could you add more comments in the description? The title is kind of abstract.

RKSimon requested changes to this revision.Jan 14 2023, 6:10 AM

@FreddyYe I've added additional tests - please can you rebase?

This revision now requires changes to proceed.Jan 14 2023, 6:10 AM
FreddyYe updated this revision to Diff 489409.Jan 15 2023, 5:59 PM

Rebase new pre-commit tests.

lebedev.ri added inline comments.Jan 15 2023, 6:06 PM
llvm/test/CodeGen/X86/prefer-fpext-splat.ll
6

Precommit please

FreddyYe edited the summary of this revision. (Show Details)Jan 15 2023, 6:06 PM
FreddyYe updated this revision to Diff 489411.Jan 15 2023, 6:37 PM

Rebase FP16 tests.

FreddyYe marked 3 inline comments as done.Jan 15 2023, 6:39 PM
RKSimon accepted this revision.Jan 16 2023, 1:58 AM

LGTM

This revision is now accepted and ready to land.Jan 16 2023, 1:58 AM
This revision was landed with ongoing or failed builds.Jan 16 2023, 6:48 AM
This revision was automatically updated to reflect the committed changes.