This is an archive of the discontinued LLVM Phabricator instance.

Differential D99219

[AArch64][GlobalISel] Mark G_SBFX/G_UBFX as legal for s32 and s64
ClosedPublic

Authored by paquette on Mar 23 2021, 3:32 PM.

Details

Reviewers
aemerson
Commits
rG1818dc394f31: [AArch64][GlobalISel] Mark G_SBFX/G_UBFX as legal for s32 and s64
Summary

This isn't perfect, since we should also verify that these only use constants...

Diff Detail

Event Timeline

paquette created this revision.Mar 23 2021, 3:32 PM
Herald added subscribers: danielkiss, hiraditya, kristof.beyls, rovka. · View Herald TranscriptMar 23 2021, 3:32 PM
paquette requested review of this revision.Mar 23 2021, 3:32 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 23 2021, 3:32 PM
aemerson added a comment.Mar 23 2021, 4:05 PM

Legalizer test?

paquette added a child revision: D99224: [AArch64][GlobalISel] Select G_SBFX and G_UBFX.Mar 23 2021, 5:05 PM
paquette updated this revision to Diff 332829.Mar 23 2021, 5:08 PM

Add legalizer tests

Harbormaster completed remote builds in B95372: Diff 332829.Mar 23 2021, 5:13 PM
Harbormaster completed remote builds in B95355: Diff 332805.Mar 23 2021, 8:04 PM
aemerson accepted this revision.Mar 23 2021, 9:50 PM
This revision is now accepted and ready to land.Mar 23 2021, 9:50 PM
This revision was landed with ongoing or failed builds.Mar 24 2021, 11:10 AM
Closed by commit rG1818dc394f31: [AArch64][GlobalISel] Mark G_SBFX/G_UBFX as legal for s32 and s64 (authored by paquette). · Explain Why
This revision was automatically updated to reflect the committed changes.
paquette added a commit: rG1818dc394f31: [AArch64][GlobalISel] Mark G_SBFX/G_UBFX as legal for s32 and s64.
arsenm added a subscriber: arsenm.Mar 24 2021, 11:11 AM
arsenm added inline comments.
llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
705

If AArch64 doesn't support these without a constant, then they can't be legal. Legality cannot be contextual. This would need to be custom, and lower if not constant to be technically correct

Revision Contents

PathSize
llvm/
lib/
Target/
AArch64/
GISel/
1 line
test/
CodeGen/
AArch64/
GlobalISel/
37 lines
37 lines
8 lines

Diff 333057

llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp

Show First 20 Lines • Show All 696 Lines • ▼ Show 20 Lines getActionDefinitionsBuilder(G_VECREDUCE_ADD)
{{s8, v16s8}, {s16, v8s16}, {s32, v4s32}, {s32, v2s32}, {s64, v2s64}}) {{s8, v16s8}, {s16, v8s16}, {s32, v4s32}, {s32, v2s32}, {s64, v2s64}})
.lower(); .lower();
getActionDefinitionsBuilder({G_UADDSAT, G_USUBSAT}) getActionDefinitionsBuilder({G_UADDSAT, G_USUBSAT})
.lowerIf([=](const LegalityQuery &Q) { return Q.Types[0].isScalar(); }); .lowerIf([=](const LegalityQuery &Q) { return Q.Types[0].isScalar(); });
getActionDefinitionsBuilder({G_FSHL, G_FSHR}).lower(); getActionDefinitionsBuilder({G_FSHL, G_FSHR}).lower();
getActionDefinitionsBuilder({G_SBFX, G_UBFX}).legalFor({s32, s64});
arsenmUnsubmitted
Not Done
ReplyInline Actions

If AArch64 doesn't support these without a constant, then they can't be legal. Legality cannot be contextual. This would need to be custom, and lower if not constant to be technically correct

arsenm: If AArch64 doesn't support these without a constant, then they can't be legal. Legality cannot…
computeTables(); computeTables();
verify(*ST.getInstrInfo()); verify(*ST.getInstrInfo());
} }
bool AArch64LegalizerInfo::legalizeCustom(LegalizerHelper &Helper, bool AArch64LegalizerInfo::legalizeCustom(LegalizerHelper &Helper,
MachineInstr &MI) const { MachineInstr &MI) const {
MachineIRBuilder &MIRBuilder = Helper.MIRBuilder; MachineIRBuilder &MIRBuilder = Helper.MIRBuilder;
MachineRegisterInfo &MRI = *MIRBuilder.getMRI(); MachineRegisterInfo &MRI = *MIRBuilder.getMRI();
▲ Show 20 Lines • Show All 237 Lines • Show Last 20 Lines

llvm/test/CodeGen/AArch64/GlobalISel/legalize-sbfx.mir

  • This file was added.
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
# RUN: llc -mtriple aarch64 -run-pass=legalizer -verify-machineinstrs %s -o - | FileCheck %s
...
---
name: s32
body: |
bb.0.entry:
liveins: $w0
; CHECK-LABEL: name: s32
; CHECK: %copy:_(s32) = COPY $w0
; CHECK: %lsb:_(s32) = G_CONSTANT i32 1
; CHECK: %width:_(s32) = G_CONSTANT i32 2
; CHECK: %sbfx:_(s32) = G_SBFX %copy, %lsb, %width
; CHECK: $w0 = COPY %sbfx(s32)
%copy:_(s32) = COPY $w0
%lsb:_(s32) = G_CONSTANT i32 1
%width:_(s32) = G_CONSTANT i32 2
%sbfx:_(s32) = G_SBFX %copy(s32), %lsb, %width
$w0 = COPY %sbfx(s32)
...
---
name: s64
body: |
bb.0.entry:
liveins: $x0
; CHECK-LABEL: name: s64
; CHECK: %copy:_(s64) = COPY $x0
; CHECK: %lsb:_(s64) = G_CONSTANT i64 1
; CHECK: %width:_(s64) = G_CONSTANT i64 2
; CHECK: %sbfx:_(s64) = G_SBFX %copy, %lsb, %width
; CHECK: $x0 = COPY %sbfx(s64)
%copy:_(s64) = COPY $x0
%lsb:_(s64) = G_CONSTANT i64 1
%width:_(s64) = G_CONSTANT i64 2
%sbfx:_(s64) = G_SBFX %copy(s64), %lsb, %width
$x0 = COPY %sbfx(s64)
...

llvm/test/CodeGen/AArch64/GlobalISel/legalize-ubfx.mir

  • This file was added.
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
# RUN: llc -mtriple aarch64 -run-pass=legalizer -verify-machineinstrs %s -o - | FileCheck %s
...
---
name: s32
body: |
bb.0.entry:
liveins: $w0
; CHECK-LABEL: name: s32
; CHECK: %copy:_(s32) = COPY $w0
; CHECK: %lsb:_(s32) = G_CONSTANT i32 1
; CHECK: %width:_(s32) = G_CONSTANT i32 2
; CHECK: %ubfx:_(s32) = G_UBFX %copy, %lsb, %width
; CHECK: $w0 = COPY %ubfx(s32)
%copy:_(s32) = COPY $w0
%lsb:_(s32) = G_CONSTANT i32 1
%width:_(s32) = G_CONSTANT i32 2
%ubfx:_(s32) = G_UBFX %copy(s32), %lsb, %width
$w0 = COPY %ubfx(s32)
...
---
name: s64
body: |
bb.0.entry:
liveins: $x0
; CHECK-LABEL: name: s64
; CHECK: %copy:_(s64) = COPY $x0
; CHECK: %lsb:_(s64) = G_CONSTANT i64 1
; CHECK: %width:_(s64) = G_CONSTANT i64 2
; CHECK: %ubfx:_(s64) = G_UBFX %copy, %lsb, %width
; CHECK: $x0 = COPY %ubfx(s64)
%copy:_(s64) = COPY $x0
%lsb:_(s64) = G_CONSTANT i64 1
%width:_(s64) = G_CONSTANT i64 2
%ubfx:_(s64) = G_UBFX %copy(s64), %lsb, %width
$x0 = COPY %ubfx(s64)
...

llvm/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir

Show First 20 Lines • Show All 668 Lines • ▼ Show 20 Lines
# DEBUG-NEXT: .. type index coverage check SKIPPED: no rules defined # DEBUG-NEXT: .. type index coverage check SKIPPED: no rules defined
# DEBUG-NEXT: .. imm index coverage check SKIPPED: no rules defined # DEBUG-NEXT: .. imm index coverage check SKIPPED: no rules defined
# DEBUG-NEXT: G_VECREDUCE_UMAX (opcode {{[0-9]+}}): 2 type indices, 0 imm indices # DEBUG-NEXT: G_VECREDUCE_UMAX (opcode {{[0-9]+}}): 2 type indices, 0 imm indices
# DEBUG-NEXT: .. type index coverage check SKIPPED: no rules defined # DEBUG-NEXT: .. type index coverage check SKIPPED: no rules defined
# DEBUG-NEXT: .. imm index coverage check SKIPPED: no rules defined # DEBUG-NEXT: .. imm index coverage check SKIPPED: no rules defined
# DEBUG-NEXT: G_VECREDUCE_UMIN (opcode {{[0-9]+}}): 2 type indices, 0 imm indices # DEBUG-NEXT: G_VECREDUCE_UMIN (opcode {{[0-9]+}}): 2 type indices, 0 imm indices
# DEBUG-NEXT: .. type index coverage check SKIPPED: no rules defined # DEBUG-NEXT: .. type index coverage check SKIPPED: no rules defined
# DEBUG-NEXT: .. imm index coverage check SKIPPED: no rules defined # DEBUG-NEXT: .. imm index coverage check SKIPPED: no rules defined
# DEBUG-NEXT: G_SBFX (opcode {{[0-9]+}}): 1 type index, 0 imm indices
# DEBUG-NEXT: .. the first uncovered type index: 1, OK
# DEBUG-NEXT: .. the first uncovered imm index: 0, OK
# DEBUG-NEXT: G_UBFX (opcode {{[0-9]+}}): 1 type index, 0 imm indices
# DEBUG-NEXT: .. opcode {{[0-9]+}} is aliased to {{[0-9]+}}
# DEBUG-NEXT: .. the first uncovered type index: 1, OK
# DEBUG-NEXT: .. the first uncovered imm index: 0, OK
# CHECK-NOT: ill-defined # CHECK-NOT: ill-defined
--- ---
name: dummy name: dummy
body: | body: |
bb.0: bb.0:
... ...