This is an archive of the discontinued LLVM Phabricator instance.

Differential D101543

[AArch64][GlobalISel] Fix width value for G_SBFX/G_UBFX
ClosedPublic

Authored by bcahoon on Apr 29 2021, 9:20 AM.

Details

Reviewers
paquette
Commits
rGd7d85f72ef9b: [AArch64][GlobalISel] Fix width value for G_SBFX/G_UBFX
Summary

When creating G_SBFX/G_UBFX opcodes, specify the last operand
as a width instead of the bit position. The bit position is used for
the AArch64 SBFM and UBFM instructions. Then, the bit position
is converted to a width if the SBFX/UBFX aliases are generated, but
the bit position is used for creating other aliases, such as the shifts.

See https://llvm.org/docs/GlobalISel/GenericOpcode.html#g-sbfx-g-ubfx
for the opcode definition.

Diff Detail

Unit TestsFailed

TimeTest
90 msx64 debian > Clang.Driver::debug-pass-structure.c

Event Timeline

bcahoon created this revision.Apr 29 2021, 9:20 AM
Herald added subscribers: danielkiss, hiraditya, kristof.beyls, rovka. · View Herald TranscriptApr 29 2021, 9:20 AM
bcahoon requested review of this revision.Apr 29 2021, 9:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 29 2021, 9:20 AM
Herald added a subscriber: llvm-commits. · View Herald Transcript
Harbormaster completed remote builds in B101660: Diff 341539.Apr 29 2021, 10:15 AM
paquette accepted this revision.Apr 29 2021, 10:36 AM

LGTM!

This revision is now accepted and ready to land.Apr 29 2021, 10:36 AM
bcahoon updated this revision to Diff 341671.Apr 29 2021, 3:04 PM

Rebase

Harbormaster completed remote builds in B101753: Diff 341671.Apr 29 2021, 5:52 PM
This revision was landed with ongoing or failed builds.Apr 29 2021, 7:05 PM
Closed by commit rGd7d85f72ef9b: [AArch64][GlobalISel] Fix width value for G_SBFX/G_UBFX (authored by bcahoon). · Explain Why
This revision was automatically updated to reflect the committed changes.
bcahoon added a commit: rGd7d85f72ef9b: [AArch64][GlobalISel] Fix width value for G_SBFX/G_UBFX.

Revision Contents

PathSize
llvm/
lib/
Target/
AArch64/
GISel/
2 lines
2 lines
test/
CodeGen/
AArch64/
GlobalISel/
4 lines
50 lines
52 lines

Diff 341539

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

Show First 20 Lines • Show All 2,320 Lines • ▼ Show 20 Lines auto Cst2 =
getConstantVRegValWithLookThrough(I.getOperand(3).getReg(), MRI); getConstantVRegValWithLookThrough(I.getOperand(3).getReg(), MRI);
assert(Cst2 && "Should have gotten a constant for src 2?"); assert(Cst2 && "Should have gotten a constant for src 2?");
auto LSB = Cst1->Value.getZExtValue(); auto LSB = Cst1->Value.getZExtValue();
auto Width = Cst2->Value.getZExtValue(); auto Width = Cst2->Value.getZExtValue();
MachineIRBuilder MIB(I); MachineIRBuilder MIB(I);
auto BitfieldInst = auto BitfieldInst =
MIB.buildInstr(Opc, {I.getOperand(0)}, {I.getOperand(1)}) MIB.buildInstr(Opc, {I.getOperand(0)}, {I.getOperand(1)})
.addImm(LSB) .addImm(LSB)
.addImm(Width); .addImm(LSB + Width - 1);
I.eraseFromParent(); I.eraseFromParent();
return constrainSelectedInstRegOperands(*BitfieldInst, TII, TRI, RBI); return constrainSelectedInstRegOperands(*BitfieldInst, TII, TRI, RBI);
} }
case TargetOpcode::G_BRCOND: case TargetOpcode::G_BRCOND:
return selectCompareBranch(I, MF, MRI); return selectCompareBranch(I, MF, MRI);
case TargetOpcode::G_BRINDIRECT: { case TargetOpcode::G_BRINDIRECT: {
I.setDesc(TII.get(AArch64::BR)); I.setDesc(TII.get(AArch64::BR));
▲ Show 20 Lines • Show All 3,789 Lines • Show Last 20 Lines

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

Show First 20 Lines • Show All 256 Lines • ▼ Show 20 Lines if (!mi_match(
Src, MRI, Src, MRI,
m_OneNonDBGUse(m_any_of(m_GAShr(m_Reg(ShiftSrc), m_ICst(ShiftImm)), m_OneNonDBGUse(m_any_of(m_GAShr(m_Reg(ShiftSrc), m_ICst(ShiftImm)),
m_GLShr(m_Reg(ShiftSrc), m_ICst(ShiftImm)))))) m_GLShr(m_Reg(ShiftSrc), m_ICst(ShiftImm))))))
return false; return false;
if (ShiftImm < 0 || ShiftImm + Width > Ty.getSizeInBits()) if (ShiftImm < 0 || ShiftImm + Width > Ty.getSizeInBits())
return false; return false;
MatchInfo = [=](MachineIRBuilder &B) { MatchInfo = [=](MachineIRBuilder &B) {
auto Cst1 = B.buildConstant(Ty, ShiftImm); auto Cst1 = B.buildConstant(Ty, ShiftImm);
auto Cst2 = B.buildConstant(Ty, ShiftImm + Width - 1); auto Cst2 = B.buildConstant(Ty, Width);
B.buildInstr(TargetOpcode::G_SBFX, {Dst}, {ShiftSrc, Cst1, Cst2}); B.buildInstr(TargetOpcode::G_SBFX, {Dst}, {ShiftSrc, Cst1, Cst2});
}; };
return true; return true;
} }
#define AARCH64POSTLEGALIZERCOMBINERHELPER_GENCOMBINERHELPER_DEPS #define AARCH64POSTLEGALIZERCOMBINERHELPER_GENCOMBINERHELPER_DEPS
#include "AArch64GenPostLegalizeGICombiner.inc" #include "AArch64GenPostLegalizeGICombiner.inc"
#undef AARCH64POSTLEGALIZERCOMBINERHELPER_GENCOMBINERHELPER_DEPS #undef AARCH64POSTLEGALIZERCOMBINERHELPER_GENCOMBINERHELPER_DEPS
▲ Show 20 Lines • Show All 112 Lines • Show Last 20 Lines

llvm/test/CodeGen/AArch64/GlobalISel/form-bitfield-extract-from-sextinreg.mir

Show All 9 Lines
legalized: true legalized: true
body: | body: |
bb.0: bb.0:
liveins: $w0 liveins: $w0
; CHECK-LABEL: name: sextinreg_ashr_to_sbfx ; CHECK-LABEL: name: sextinreg_ashr_to_sbfx
; CHECK: liveins: $w0 ; CHECK: liveins: $w0
; CHECK: %x:_(s32) = COPY $w0 ; CHECK: %x:_(s32) = COPY $w0
; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 5 ; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 5
; CHECK: [[C1:%[0-9]+]]:_(s32) = G_CONSTANT i32 14 ; CHECK: [[C1:%[0-9]+]]:_(s32) = G_CONSTANT i32 10
; CHECK: %sext_inreg:_(s32) = G_SBFX %x, [[C]](s32), [[C1]] ; CHECK: %sext_inreg:_(s32) = G_SBFX %x, [[C]](s32), [[C1]]
; CHECK: $w0 = COPY %sext_inreg(s32) ; CHECK: $w0 = COPY %sext_inreg(s32)
; CHECK: RET_ReallyLR implicit $w0 ; CHECK: RET_ReallyLR implicit $w0
%x:_(s32) = COPY $w0 %x:_(s32) = COPY $w0
%lsb:_(s32) = G_CONSTANT i32 5 %lsb:_(s32) = G_CONSTANT i32 5
%shift:_(s32) = G_ASHR %x, %lsb %shift:_(s32) = G_ASHR %x, %lsb
%sext_inreg:_(s32) = G_SEXT_INREG %shift, 10 %sext_inreg:_(s32) = G_SEXT_INREG %shift, 10
$w0 = COPY %sext_inreg $w0 = COPY %sext_inreg
RET_ReallyLR implicit $w0 RET_ReallyLR implicit $w0
... ...
--- ---
name: sextinreg_lshr_to_sbfx name: sextinreg_lshr_to_sbfx
tracksRegLiveness: true tracksRegLiveness: true
legalized: true legalized: true
body: | body: |
bb.0: bb.0:
liveins: $w0 liveins: $w0
; CHECK-LABEL: name: sextinreg_lshr_to_sbfx ; CHECK-LABEL: name: sextinreg_lshr_to_sbfx
; CHECK: liveins: $w0 ; CHECK: liveins: $w0
; CHECK: %x:_(s32) = COPY $w0 ; CHECK: %x:_(s32) = COPY $w0
; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 5 ; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 5
; CHECK: [[C1:%[0-9]+]]:_(s32) = G_CONSTANT i32 14 ; CHECK: [[C1:%[0-9]+]]:_(s32) = G_CONSTANT i32 10
; CHECK: %sext_inreg:_(s32) = G_SBFX %x, [[C]](s32), [[C1]] ; CHECK: %sext_inreg:_(s32) = G_SBFX %x, [[C]](s32), [[C1]]
; CHECK: $w0 = COPY %sext_inreg(s32) ; CHECK: $w0 = COPY %sext_inreg(s32)
; CHECK: RET_ReallyLR implicit $w0 ; CHECK: RET_ReallyLR implicit $w0
%x:_(s32) = COPY $w0 %x:_(s32) = COPY $w0
%lsb:_(s32) = G_CONSTANT i32 5 %lsb:_(s32) = G_CONSTANT i32 5
%shift:_(s32) = G_LSHR %x, %lsb %shift:_(s32) = G_LSHR %x, %lsb
%sext_inreg:_(s32) = G_SEXT_INREG %shift, 10 %sext_inreg:_(s32) = G_SEXT_INREG %shift, 10
$w0 = COPY %sext_inreg $w0 = COPY %sext_inreg
▲ Show 20 Lines • Show All 104 Lines • Show Last 20 Lines

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

# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
# RUN: llc -mtriple=aarch64 -run-pass=instruction-select -verify-machineinstrs %s -o - | FileCheck %s # RUN: llc -mtriple=aarch64 -run-pass=instruction-select -verify-machineinstrs %s -o - | FileCheck %s
... ...
--- ---
name: sbfx_s32 name: sbfx_s32
legalized: true legalized: true
regBankSelected: true regBankSelected: true
tracksRegLiveness: true tracksRegLiveness: true
body: | body: |
bb.0: bb.0:
liveins: $w0 liveins: $w0
; CHECK-LABEL: name: sbfx_s32 ; CHECK-LABEL: name: sbfx_s32
; CHECK: liveins: $w0 ; CHECK: liveins: $w0
; CHECK: %copy:gpr32 = COPY $w0 ; CHECK: %copy:gpr32 = COPY $w0
; CHECK: %sbfx:gpr32 = SBFMWri %copy, 0, 10 ; CHECK: %sbfx:gpr32 = SBFMWri %copy, 0, 9
; CHECK: $w0 = COPY %sbfx ; CHECK: $w0 = COPY %sbfx
; CHECK: RET_ReallyLR implicit $w0 ; CHECK: RET_ReallyLR implicit $w0
%copy:gpr(s32) = COPY $w0 %copy:gpr(s32) = COPY $w0
%cst1:gpr(s32) = G_CONSTANT i32 0 %cst1:gpr(s32) = G_CONSTANT i32 0
%cst2:gpr(s32) = G_CONSTANT i32 10 %cst2:gpr(s32) = G_CONSTANT i32 10
%sbfx:gpr(s32) = G_SBFX %copy, %cst1, %cst2 %sbfx:gpr(s32) = G_SBFX %copy, %cst1, %cst2
$w0 = COPY %sbfx $w0 = COPY %sbfx
RET_ReallyLR implicit $w0 RET_ReallyLR implicit $w0
... ...
--- ---
name: sbfx_s64 name: sbfx_s64
legalized: true legalized: true
regBankSelected: true regBankSelected: true
tracksRegLiveness: true tracksRegLiveness: true
body: | body: |
bb.0: bb.0:
liveins: $x0 liveins: $x0
; CHECK-LABEL: name: sbfx_s64 ; CHECK-LABEL: name: sbfx_s64
; CHECK: liveins: $x0 ; CHECK: liveins: $x0
; CHECK: %copy:gpr64 = COPY $x0 ; CHECK: %copy:gpr64 = COPY $x0
; CHECK: %sbfx:gpr64 = SBFMXri %copy, 0, 10 ; CHECK: %sbfx:gpr64 = SBFMXri %copy, 0, 9
; CHECK: $x0 = COPY %sbfx ; CHECK: $x0 = COPY %sbfx
; CHECK: RET_ReallyLR implicit $x0 ; CHECK: RET_ReallyLR implicit $x0
%copy:gpr(s64) = COPY $x0 %copy:gpr(s64) = COPY $x0
%cst1:gpr(s64) = G_CONSTANT i64 0 %cst1:gpr(s64) = G_CONSTANT i64 0
%cst2:gpr(s64) = G_CONSTANT i64 10 %cst2:gpr(s64) = G_CONSTANT i64 10
%sbfx:gpr(s64) = G_SBFX %copy, %cst1, %cst2 %sbfx:gpr(s64) = G_SBFX %copy, %cst1, %cst2
$x0 = COPY %sbfx $x0 = COPY %sbfx
RET_ReallyLR implicit $x0 RET_ReallyLR implicit $x0
... ...
--- ---
name: sbfx_s32_31_31 name: sbfx_s32_31_1
legalized: true legalized: true
regBankSelected: true regBankSelected: true
tracksRegLiveness: true tracksRegLiveness: true
body: | body: |
bb.0: bb.0:
liveins: $w0 liveins: $w0
; This is just an asr, so it's okay. ; This is just an asr, so it's okay.
; CHECK-LABEL: name: sbfx_s32_31_31 ; CHECK-LABEL: name: sbfx_s32_31_1
; CHECK: liveins: $w0 ; CHECK: liveins: $w0
; CHECK: %copy:gpr32 = COPY $w0 ; CHECK: %copy:gpr32 = COPY $w0
; CHECK: %sbfx:gpr32 = SBFMWri %copy, 31, 31 ; CHECK: %sbfx:gpr32 = SBFMWri %copy, 31, 31
; CHECK: $w0 = COPY %sbfx ; CHECK: $w0 = COPY %sbfx
; CHECK: RET_ReallyLR implicit $w0 ; CHECK: RET_ReallyLR implicit $w0
%copy:gpr(s32) = COPY $w0 %copy:gpr(s32) = COPY $w0
%cst1:gpr(s32) = G_CONSTANT i32 31 %cst1:gpr(s32) = G_CONSTANT i32 31
%cst2:gpr(s32) = G_CONSTANT i32 31 %cst2:gpr(s32) = G_CONSTANT i32 1
%sbfx:gpr(s32) = G_SBFX %copy, %cst1, %cst2 %sbfx:gpr(s32) = G_SBFX %copy, %cst1, %cst2
$w0 = COPY %sbfx $w0 = COPY %sbfx
RET_ReallyLR implicit $w0 RET_ReallyLR implicit $w0
---
name: sbfx_s32_10_5
legalized: true
regBankSelected: true
tracksRegLiveness: true
body: |
bb.0:
liveins: $w0
; CHECK-LABEL: name: sbfx_s32_10_5
; CHECK: liveins: $w0
; CHECK: %copy:gpr32 = COPY $w0
; CHECK: %sbfx:gpr32 = SBFMWri %copy, 10, 14
; CHECK: $w0 = COPY %sbfx
; CHECK: RET_ReallyLR implicit $w0
%copy:gpr(s32) = COPY $w0
%cst1:gpr(s32) = G_CONSTANT i32 10
%cst2:gpr(s32) = G_CONSTANT i32 5
%sbfx:gpr(s32) = G_SBFX %copy, %cst1, %cst2
$w0 = COPY %sbfx
RET_ReallyLR implicit $w0
---
name: sbfx_s64_10_5
legalized: true
regBankSelected: true
tracksRegLiveness: true
body: |
bb.0:
liveins: $x0
; CHECK-LABEL: name: sbfx_s64_10_5
; CHECK: liveins: $x0
; CHECK: %copy:gpr64 = COPY $x0
; CHECK: %sbfx:gpr64 = SBFMXri %copy, 10, 14
; CHECK: $x0 = COPY %sbfx
; CHECK: RET_ReallyLR implicit $x0
%copy:gpr(s64) = COPY $x0
%cst1:gpr(s64) = G_CONSTANT i64 10
%cst2:gpr(s64) = G_CONSTANT i64 5
%sbfx:gpr(s64) = G_SBFX %copy, %cst1, %cst2
$x0 = COPY %sbfx
RET_ReallyLR implicit $x0

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

# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
# RUN: llc -mtriple=aarch64 -run-pass=instruction-select -verify-machineinstrs %s -o - | FileCheck %s # RUN: llc -mtriple=aarch64 -run-pass=instruction-select -verify-machineinstrs %s -o - | FileCheck %s
... ...
--- ---
name: ubfx_s32 name: ubfx_s32
legalized: true legalized: true
regBankSelected: true regBankSelected: true
tracksRegLiveness: true tracksRegLiveness: true
body: | body: |
bb.0: bb.0:
liveins: $w0 liveins: $w0
; CHECK-LABEL: name: ubfx_s32 ; CHECK-LABEL: name: ubfx_s32
; CHECK: liveins: $w0 ; CHECK: liveins: $w0
; CHECK: %copy:gpr32 = COPY $w0 ; CHECK: %copy:gpr32 = COPY $w0
; CHECK: %ubfx:gpr32 = UBFMWri %copy, 0, 10 ; CHECK: %ubfx:gpr32 = UBFMWri %copy, 0, 9
; CHECK: $w0 = COPY %ubfx ; CHECK: $w0 = COPY %ubfx
; CHECK: RET_ReallyLR implicit $w0 ; CHECK: RET_ReallyLR implicit $w0
%copy:gpr(s32) = COPY $w0 %copy:gpr(s32) = COPY $w0
%cst1:gpr(s32) = G_CONSTANT i32 0 %cst1:gpr(s32) = G_CONSTANT i32 0
%cst2:gpr(s32) = G_CONSTANT i32 10 %cst2:gpr(s32) = G_CONSTANT i32 10
%ubfx:gpr(s32) = G_UBFX %copy, %cst1, %cst2 %ubfx:gpr(s32) = G_UBFX %copy, %cst1, %cst2
$w0 = COPY %ubfx $w0 = COPY %ubfx
RET_ReallyLR implicit $w0 RET_ReallyLR implicit $w0
... ...
--- ---
name: ubfx_s64 name: ubfx_s64
legalized: true legalized: true
regBankSelected: true regBankSelected: true
tracksRegLiveness: true tracksRegLiveness: true
body: | body: |
bb.0: bb.0:
liveins: $x0 liveins: $x0
; CHECK-LABEL: name: ubfx_s64 ; CHECK-LABEL: name: ubfx_s64
; CHECK: liveins: $x0 ; CHECK: liveins: $x0
; CHECK: %copy:gpr64 = COPY $x0 ; CHECK: %copy:gpr64 = COPY $x0
; CHECK: %ubfx:gpr64 = UBFMXri %copy, 0, 10 ; CHECK: %ubfx:gpr64 = UBFMXri %copy, 0, 9
; CHECK: $x0 = COPY %ubfx ; CHECK: $x0 = COPY %ubfx
; CHECK: RET_ReallyLR implicit $x0 ; CHECK: RET_ReallyLR implicit $x0
%copy:gpr(s64) = COPY $x0 %copy:gpr(s64) = COPY $x0
%cst1:gpr(s64) = G_CONSTANT i64 0 %cst1:gpr(s64) = G_CONSTANT i64 0
%cst2:gpr(s64) = G_CONSTANT i64 10 %cst2:gpr(s64) = G_CONSTANT i64 10
%ubfx:gpr(s64) = G_UBFX %copy, %cst1, %cst2 %ubfx:gpr(s64) = G_UBFX %copy, %cst1, %cst2
$x0 = COPY %ubfx $x0 = COPY %ubfx
RET_ReallyLR implicit $x0 RET_ReallyLR implicit $x0
... ...
--- ---
name: ubfx_s32_31_31 name: ubfx_s32_31_1
legalized: true legalized: true
regBankSelected: true regBankSelected: true
tracksRegLiveness: true tracksRegLiveness: true
body: | body: |
bb.0: bb.0:
liveins: $w0 liveins: $w0
; This is just a lsr, so it's okay. ; This is just a lsr, so it's okay.
; CHECK-LABEL: name: ubfx_s32_31_31 ; CHECK-LABEL: name: ubfx_s32_31_1
; CHECK: liveins: $w0 ; CHECK: liveins: $w0
; CHECK: %copy:gpr32 = COPY $w0 ; CHECK: %copy:gpr32 = COPY $w0
; CHECK: %ubfx:gpr32 = UBFMWri %copy, 31, 31 ; CHECK: %ubfx:gpr32 = UBFMWri %copy, 31, 31
; CHECK: $w0 = COPY %ubfx ; CHECK: $w0 = COPY %ubfx
; CHECK: RET_ReallyLR implicit $w0 ; CHECK: RET_ReallyLR implicit $w0
%copy:gpr(s32) = COPY $w0 %copy:gpr(s32) = COPY $w0
%cst1:gpr(s32) = G_CONSTANT i32 31 %cst1:gpr(s32) = G_CONSTANT i32 31
%cst2:gpr(s32) = G_CONSTANT i32 31 %cst2:gpr(s32) = G_CONSTANT i32 1
%ubfx:gpr(s32) = G_UBFX %copy, %cst1, %cst2 %ubfx:gpr(s32) = G_UBFX %copy, %cst1, %cst2
$w0 = COPY %ubfx $w0 = COPY %ubfx
RET_ReallyLR implicit $w0 RET_ReallyLR implicit $w0
---
name: ubfx_s32_10_5
legalized: true
regBankSelected: true
tracksRegLiveness: true
body: |
bb.0:
liveins: $w0
; CHECK-LABEL: name: ubfx_s32_10_5
; CHECK: liveins: $w0
; CHECK: %copy:gpr32 = COPY $w0
; CHECK: %ubfx:gpr32 = UBFMWri %copy, 10, 14
; CHECK: $w0 = COPY %ubfx
; CHECK: RET_ReallyLR implicit $w0
%copy:gpr(s32) = COPY $w0
%cst1:gpr(s32) = G_CONSTANT i32 10
%cst2:gpr(s32) = G_CONSTANT i32 5
%ubfx:gpr(s32) = G_UBFX %copy, %cst1, %cst2
$w0 = COPY %ubfx
RET_ReallyLR implicit $w0
...
---
name: ubfx_s64_10_5
legalized: true
regBankSelected: true
tracksRegLiveness: true
body: |
bb.0:
liveins: $x0
; CHECK-LABEL: name: ubfx_s64_10_5
; CHECK: liveins: $x0
; CHECK: %copy:gpr64 = COPY $x0
; CHECK: %ubfx:gpr64 = UBFMXri %copy, 10, 14
; CHECK: $x0 = COPY %ubfx
; CHECK: RET_ReallyLR implicit $x0
%copy:gpr(s64) = COPY $x0
%cst1:gpr(s64) = G_CONSTANT i64 10
%cst2:gpr(s64) = G_CONSTANT i64 5
%ubfx:gpr(s64) = G_UBFX %copy, %cst1, %cst2
$x0 = COPY %ubfx
RET_ReallyLR implicit $x0