Skip to content

[X86][GISel] Add missing legalization for G_IMPLICIT_DEF #161699

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

Merged
merged 6 commits into from
Oct 6, 2025
Merged

[X86][GISel] Add missing legalization for G_IMPLICIT_DEF #161699

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
81e18b2
[X86][GISel] Add missing legalization for G_IMPLICIT_DEF
bonsthie Oct 2, 2025
9decd8a
Merge branch 'main' into x86-gisel-legalizer-implicit-def
bonsthie Oct 2, 2025
0024770
[X86][GISel] Unify AVX/AVX2 and AVX512/AVX512BW type legality
bonsthie Oct 3, 2025
381281a
[X86][GISel] Merge builders for G_IMPLICIT_DEF, G_PHI, G_FREEZE, and …
bonsthie Oct 3, 2025
614e875
[X86][GISel] Support G_FREEZE and G_CONSTANT_FOLD_BARRIER selection
bonsthie Oct 4, 2025
5d2aba5
Merge branch 'main' into x86-gisel-legalizer-implicit-def
RKSimon Oct 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion llvm/lib/Target/X86/GISel/X86LegalizerInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,27 @@ X86LegalizerInfo::X86LegalizerInfo(const X86Subtarget &STI,
// s128 = EXTEND (G_IMPLICIT_DEF s32/s64) -> s128 = G_IMPLICIT_DEF
getActionDefinitionsBuilder(G_IMPLICIT_DEF)
.legalFor({p0, s1, s8, s16, s32, s64})
.legalFor(Is64Bit, {s128});
.legalFor(Is64Bit, {s128})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

G_PHI, G_IMPLICIT_DEF, and G_FREEZE (and a few others) should have identical rule sets

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean grouping them under the same ActionDefinitionsBuilder so they share an identical rule set?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. We probably should enforce this somehow. We have an implicit notion of legal types, which we would be better off if we formalized and had the machine verifier enforce. It's certainly weaker than the DAG case, but we do need to guarantee a set of properties to ensure the legalizer can complete

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now I will fuse them in the legalizer, and I’ll look to see how we can enforce this in the machine verifier.

.legalFor(HasSSE2, {v16s8, v8s16, v4s32, v2s64})
.legalFor(HasAVX, {v8s32, v4s64})
.legalFor(HasAVX2, {v32s8, v16s16, v8s32, v4s64})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HasAVX/AVX2 can be merged into a single .legalFor(HasAVX, {v32s8, v16s16, v8s32, v4s64}) - as all these types are legal on AVX1

Similar for clamping below

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I split them because AVX1 lacks 256-bit integer ops, but you’re right that for legalization we only care that the register types exist in the ISA.

.legalFor(HasAVX512, {v16s32, v8s64})
.legalFor(HasBWI, {v64s8, v32s16})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as AVX1/2 - HasAVX512 supports all 512-bit vector types

.widenScalarOrEltToNextPow2(0, /*Min=*/8)
.clampScalarOrElt(0, s8, sMaxScalar)
.moreElementsToNextPow2(0)
.clampMinNumElements(0, s8, 16)
.clampMinNumElements(0, s16, 8)
.clampMinNumElements(0, s32, 4)
.clampMinNumElements(0, s64, 2)
.clampMaxNumElements(0, s8, HasBWI ? 64 : (HasAVX2 ? 32 : 16))
.clampMaxNumElements(0, s16, HasBWI ? 32 : (HasAVX2 ? 16 : 8))
.clampMaxNumElements(0, s32, HasAVX512 ? 16 : (HasAVX2 ? 8 : 4))
.clampMaxNumElements(0, s64, HasAVX512 ? 8 : (HasAVX2 ? 4 : 2))
.clampMaxNumElements(0, p0,
Is64Bit ? s64MaxVector.getNumElements()
: s32MaxVector.getNumElements())
.scalarizeIf(scalarOrEltWiderThan(0, 64), 0);

getActionDefinitionsBuilder(G_CONSTANT)
.legalFor({p0, s8, s16, s32})
Expand Down
32 changes: 32 additions & 0 deletions llvm/test/CodeGen/X86/GlobalISel/legalize-g_implicit_def.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# RUN: llc -mtriple=x86_64-linux-gnu -mattr=avx2 -run-pass=legalizer -global-isel-abort=2 -pass-remarks-missed='gisel*' %s -o - | FileCheck %s --check-prefixes=CHECK,AVX2
# RUN: llc -mtriple=x86_64-linux-gnu -mattr=sse2 -run-pass=legalizer -global-isel-abort=2 -pass-remarks-missed='gisel*' %s -o - | FileCheck %s --check-prefixes=CHECK,SSE2
# RUN: llc -mtriple=x86_64-linux-gnu -mattr=avx512f -run-pass=legalizer -global-isel-abort=2 -pass-remarks-missed='gisel*' %s -o - | FileCheck %s --check-prefixes=CHECK,AVX512F


---
name: test_basic_g_implicit_def_v8i64
body: |
bb.0:
; CHECK-LABEL: name: test_basic_g_implicit_def_v8i64
; AVX512F: {{%[0-9]+}}:_(<8 x s64>) = G_IMPLICIT_DEF
; AVX2: [[DEF_AVX2:%[0-9]+]]:_(<4 x s64>) = G_IMPLICIT_DEF
; AVX2-NEXT: {{%[0-9]+}}:_(<8 x s64>) = G_CONCAT_VECTORS [[DEF_AVX2]](<4 x s64>), [[DEF_AVX2]](<4 x s64>)
; SSE2: [[DEF_SSE2:%[0-9]+]]:_(<2 x s64>) = G_IMPLICIT_DEF
; SSE2-NEXT: {{%[0-9]+}}:_(<8 x s64>) = G_CONCAT_VECTORS [[DEF_SSE2]](<2 x s64>), [[DEF_SSE2]](<2 x s64>), [[DEF_SSE2]](<2 x s64>), [[DEF_SSE2]](<2 x s64>)
%0:_(<8 x s64>) = G_IMPLICIT_DEF
RET 0, implicit %0
...

---
name: test_g_implicit_def_cample_size
body: |
bb.1:
; CHECK-LABEL: name: test_g_implicit_def_cample_size
; AVX512: {{%[0-9]+}}:_(<8 x s64>) = G_IMPLICIT_DEF
; AVX2: {{%[0-9]+}}:_(<4 x s64>) = G_IMPLICIT_DEF
; SSE2: {{%[0-9]+}}:_(<2 x s64>) = G_IMPLICIT_DEF
%0:_(<5 x s63>) = G_IMPLICIT_DEF
RET 0, implicit %0
...