This is an archive of the discontinued LLVM Phabricator instance.

Differential D11592

[mips][FastISel] Apply only zero-extension to constants prior to their materialization.
ClosedPublic

Authored by vkalintiris on Jul 29 2015, 6:52 AM.

Details

Reviewers
dsanders
Commits
rG7cb5d069d325: Merging r243636: --------------------------------------------------------------…
rG77fb0a3dcf19: [mips][FastISel] Apply only zero-extension to constants prior to their…
rL243636: [mips][FastISel] Apply only zero-extension to constants prior to their…
Summary

Previously, we would sign-extend non-boolean negative constants and
zero-extend otherwise. This was problematic for PHI instructions with
negative values that had a type with bitwidth less than that of the
register used for materialization.

More specifically, ComputePHILiveOutRegInfo() zero-extends (or
truncates to the bitwidth of destination-type) the constants present
in a PHI node, and afterwards deduces the known bits.

For example, previously we would materialize an i16 -4 with the
following instruction:

addiu $r, $zero, -4

The register would end-up with the 32-bit 2's complement representation
of -4. However, ComputePHILiveOutRegInfo() would generate a constant
with the upper 16-bits set to zero. The SelectionDAG builder would use
that information to generate an AzzertZero node that would remove any
subsequent trunc & zero_extend nodes.

In theory, we should modify ComputePHILiveOutRegInfo() to consult
target-specific hooks about the way they prefer to materialize the
given constants. However, git-blame reports that this specific code
has not been touched since 2011 and it seems to be working well for every
target so far.

Diff Detail

Repository
rL LLVM

Event Timeline

vkalintiris updated this revision to Diff 30904.Jul 29 2015, 6:52 AM
vkalintiris retitled this revision from to [mips][FastISel] Apply only zero-extension to constants prior to their materialization..
vkalintiris updated this object.
vkalintiris added a reviewer: dsanders.
vkalintiris added a subscriber: llvm-commits.
vkalintiris mentioned this in D11516: [mips][FastISel] Fix code generation for logical-ops by zero-extending constant operands..Jul 29 2015, 7:38 AM
dsanders accepted this revision.Jul 30 2015, 4:18 AM
dsanders edited edge metadata.

LGTM. I agree that the real issue is that the analysis of known bits is getting the excess bits wrong and we ought to fix that but this patch at least fixes the known issues in Mips's Fast ISel. We can follow up on the known-bits analysis afterwards.

More specifically, ComputePHILiveOutRegInfo() zero-extends (or
truncates to the bitwidth of destination-type) ...

These aren't quite the same thing. Is it assuming values are zero extended in their container or is considering the excess bits to be undefined? From your later comments I'm sure it's the former.

AzzertZero

Nit: Typo

This revision is now accepted and ready to land.Jul 30 2015, 4:18 AM
Closed by commit rL243636: [mips][FastISel] Apply only zero-extension to constants prior to their… (authored by vkalintiris). · Explain WhyJul 30 2015, 4:52 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
llvm/
trunk/
lib/
Target/
Mips/
7 lines
test/
CodeGen/
Mips/
Fast-ISel/
20 lines
7 lines

Diff 31016

llvm/trunk/lib/Target/Mips/MipsFastISel.cpp

Show First 20 Lines • Show All 284 Lines • ▼ Show 20 Linesunsigned MipsFastISel::fastMaterializeAlloca(const AllocaInst *AI) {
return 0; return 0;
} }
unsigned MipsFastISel::materializeInt(const Constant *C, MVT VT) { unsigned MipsFastISel::materializeInt(const Constant *C, MVT VT) {
if (VT != MVT::i32 && VT != MVT::i16 && VT != MVT::i8 && VT != MVT::i1) if (VT != MVT::i32 && VT != MVT::i16 && VT != MVT::i8 && VT != MVT::i1)
return 0; return 0;
const TargetRegisterClass *RC = &Mips::GPR32RegClass; const TargetRegisterClass *RC = &Mips::GPR32RegClass;
const ConstantInt *CI = cast<ConstantInt>(C); const ConstantInt *CI = cast<ConstantInt>(C);
int64_t Imm; return materialize32BitInt(CI->getZExtValue(), RC);
if ((VT != MVT::i1) && CI->isNegative())
Imm = CI->getSExtValue();
else
Imm = CI->getZExtValue();
return materialize32BitInt(Imm, RC);
} }
unsigned MipsFastISel::materialize32BitInt(int64_t Imm, unsigned MipsFastISel::materialize32BitInt(int64_t Imm,
const TargetRegisterClass *RC) { const TargetRegisterClass *RC) {
unsigned ResultReg = createResultReg(RC); unsigned ResultReg = createResultReg(RC);
if (isInt<16>(Imm)) { if (isInt<16>(Imm)) {
unsigned Opc = Mips::ADDiu; unsigned Opc = Mips::ADDiu;
▲ Show 20 Lines • Show All 1,571 Lines • Show Last 20 Lines

llvm/trunk/test/CodeGen/Mips/Fast-ISel/logopm.ll

Show First 20 Lines • Show All 277 Lines • ▼ Show 20 Linesentry:
store i8 %and, i8* @uc, align 1, !tbaa !2 store i8 %and, i8* @uc, align 1, !tbaa !2
; CHECK-LABEL: .ent andUc1 ; CHECK-LABEL: .ent andUc1
; CHECK: lui $[[REG_GPa:[0-9]+]], %hi(_gp_disp) ; CHECK: lui $[[REG_GPa:[0-9]+]], %hi(_gp_disp)
; CHECK: addiu $[[REG_GPb:[0-9]+]], $[[REG_GPa]], %lo(_gp_disp) ; CHECK: addiu $[[REG_GPb:[0-9]+]], $[[REG_GPa]], %lo(_gp_disp)
; CHECK: addu $[[REG_GP:[0-9]+]], $[[REG_GPb]], $25 ; CHECK: addu $[[REG_GP:[0-9]+]], $[[REG_GPb]], $25
; CHECK-DAG: lw $[[UC_ADDR:[0-9]+]], %got(uc)($[[REG_GP]]) ; CHECK-DAG: lw $[[UC_ADDR:[0-9]+]], %got(uc)($[[REG_GP]])
; CHECK-DAG: lw $[[UC1_ADDR:[0-9]+]], %got(uc1)($[[REG_GP]]) ; CHECK-DAG: lw $[[UC1_ADDR:[0-9]+]], %got(uc1)($[[REG_GP]])
; CHECK-DAG: lbu $[[UC1:[0-9]+]], 0($[[UC1_ADDR]]) ; CHECK-DAG: lbu $[[UC1:[0-9]+]], 0($[[UC1_ADDR]])
; CHECK-DAG: addiu $[[CONST_Neg89:[0-9]+]], $zero, -89 ; CHECK-DAG: addiu $[[CONST_167:[0-9]+]], $zero, 167
; CHECK-DAG: and $[[RES:[0-9]+]], $[[UC1]], $[[CONST_Neg89]] ; CHECK-DAG: and $[[RES:[0-9]+]], $[[UC1]], $[[CONST_167]]
; CHECK: sb $[[RES]], 0($[[UC_ADDR]]) ; CHECK: sb $[[RES]], 0($[[UC_ADDR]])
; CHECK: .end andUc1 ; CHECK: .end andUc1
ret void ret void
} }
; Function Attrs: noinline nounwind ; Function Attrs: noinline nounwind
define void @orUc() #0 { define void @orUc() #0 {
entry: entry:
▲ Show 20 Lines • Show All 44 Lines • ▼ Show 20 Linesentry:
store i8 %or, i8* @uc, align 1, !tbaa !2 store i8 %or, i8* @uc, align 1, !tbaa !2
; CHECK-LABEL: .ent orUc1 ; CHECK-LABEL: .ent orUc1
; CHECK: lui $[[REG_GPa:[0-9]+]], %hi(_gp_disp) ; CHECK: lui $[[REG_GPa:[0-9]+]], %hi(_gp_disp)
; CHECK: addiu $[[REG_GPb:[0-9]+]], $[[REG_GPa]], %lo(_gp_disp) ; CHECK: addiu $[[REG_GPb:[0-9]+]], $[[REG_GPa]], %lo(_gp_disp)
; CHECK: addu $[[REG_GP:[0-9]+]], $[[REG_GPb]], $25 ; CHECK: addu $[[REG_GP:[0-9]+]], $[[REG_GPb]], $25
; CHECK-DAG: lw $[[UC_ADDR:[0-9]+]], %got(uc)($[[REG_GP]]) ; CHECK-DAG: lw $[[UC_ADDR:[0-9]+]], %got(uc)($[[REG_GP]])
; CHECK-DAG: lw $[[UC1_ADDR:[0-9]+]], %got(uc1)($[[REG_GP]]) ; CHECK-DAG: lw $[[UC1_ADDR:[0-9]+]], %got(uc1)($[[REG_GP]])
; CHECK-DAG: lbu $[[UC1:[0-9]+]], 0($[[UC1_ADDR]]) ; CHECK-DAG: lbu $[[UC1:[0-9]+]], 0($[[UC1_ADDR]])
; CHECK-DAG: addiu $[[CONST_neg18:[0-9]+]], $zero, -18 ; CHECK-DAG: addiu $[[CONST_238:[0-9]+]], $zero, 238
; CHECK-DAG: or $[[RES:[0-9]+]], $[[UC1]], $[[CONST_neg18]] ; CHECK-DAG: or $[[RES:[0-9]+]], $[[UC1]], $[[CONST_238]]
; CHECK: sb $[[RES]], 0($[[UC_ADDR]]) ; CHECK: sb $[[RES]], 0($[[UC_ADDR]])
; CHECK: .end orUc1 ; CHECK: .end orUc1
ret void ret void
} }
; Function Attrs: noinline nounwind ; Function Attrs: noinline nounwind
define void @xorUc() #0 { define void @xorUc() #0 {
entry: entry:
▲ Show 20 Lines • Show All 106 Lines • ▼ Show 20 Linesentry:
store i16 %and, i16* @us, align 2, !tbaa !5 store i16 %and, i16* @us, align 2, !tbaa !5
; CHECK-LABEL: .ent andUs1 ; CHECK-LABEL: .ent andUs1
; CHECK: lui $[[REG_GPa:[0-9]+]], %hi(_gp_disp) ; CHECK: lui $[[REG_GPa:[0-9]+]], %hi(_gp_disp)
; CHECK: addiu $[[REG_GPb:[0-9]+]], $[[REG_GPa]], %lo(_gp_disp) ; CHECK: addiu $[[REG_GPb:[0-9]+]], $[[REG_GPa]], %lo(_gp_disp)
; CHECK: addu $[[REG_GP:[0-9]+]], $[[REG_GPb]], $25 ; CHECK: addu $[[REG_GP:[0-9]+]], $[[REG_GPb]], $25
; CHECK-DAG: lw $[[US_ADDR:[0-9]+]], %got(us)($[[REG_GP]]) ; CHECK-DAG: lw $[[US_ADDR:[0-9]+]], %got(us)($[[REG_GP]])
; CHECK-DAG: lw $[[US1_ADDR:[0-9]+]], %got(us1)($[[REG_GP]]) ; CHECK-DAG: lw $[[US1_ADDR:[0-9]+]], %got(us1)($[[REG_GP]])
; CHECK-DAG: lhu $[[US1:[0-9]+]], 0($[[US1_ADDR]]) ; CHECK-DAG: lhu $[[US1:[0-9]+]], 0($[[US1_ADDR]])
; CHECK-DAG: addiu $[[CONST_Neg4185:[0-9]+]], $zero, -4185 ; CHECK-DAG: ori $[[CONST_61351:[0-9]+]], $zero, 61351
; CHECK-DAG: and $[[RES:[0-9]+]], $[[US1]], $[[CONST_Neg4185]] ; CHECK-DAG: and $[[RES:[0-9]+]], $[[US1]], $[[CONST_61351]]
; CHECK: sh $[[RES]], 0($[[US_ADDR]]) ; CHECK: sh $[[RES]], 0($[[US_ADDR]])
; CHECK: .end andUs1 ; CHECK: .end andUs1
ret void ret void
} }
; Function Attrs: noinline nounwind ; Function Attrs: noinline nounwind
define void @orUs() #0 { define void @orUs() #0 {
entry: entry:
Show All 33 Linesentry:
store i16 %or, i16* @us, align 2, !tbaa !5 store i16 %or, i16* @us, align 2, !tbaa !5
; CHECK-LABEL: .ent orUs1 ; CHECK-LABEL: .ent orUs1
; CHECK: lui $[[REG_GPa:[0-9]+]], %hi(_gp_disp) ; CHECK: lui $[[REG_GPa:[0-9]+]], %hi(_gp_disp)
; CHECK: addiu $[[REG_GPb:[0-9]+]], $[[REG_GPa]], %lo(_gp_disp) ; CHECK: addiu $[[REG_GPb:[0-9]+]], $[[REG_GPa]], %lo(_gp_disp)
; CHECK: addu $[[REG_GP:[0-9]+]], $[[REG_GPb]], $25 ; CHECK: addu $[[REG_GP:[0-9]+]], $[[REG_GPb]], $25
; CHECK-DAG: lw $[[US_ADDR:[0-9]+]], %got(us)($[[REG_GP]]) ; CHECK-DAG: lw $[[US_ADDR:[0-9]+]], %got(us)($[[REG_GP]])
; CHECK-DAG: lw $[[US1_ADDR:[0-9]+]], %got(us1)($[[REG_GP]]) ; CHECK-DAG: lw $[[US1_ADDR:[0-9]+]], %got(us1)($[[REG_GP]])
; CHECK-DAG: lhu $[[US1:[0-9]+]], 0($[[US1_ADDR]]) ; CHECK-DAG: lhu $[[US1:[0-9]+]], 0($[[US1_ADDR]])
; CHECK-DAG: addiu $[[CONST_neg4591:[0-9]+]], $zero, -4591 ; CHECK-DAG: ori $[[CONST_60945:[0-9]+]], $zero, 60945
; CHECK-DAG: or $[[RES:[0-9]+]], $[[US1]], $[[CONST_neg4591]] ; CHECK-DAG: or $[[RES:[0-9]+]], $[[US1]], $[[CONST_60945]]
; CHECK: sh $[[RES]], 0($[[US_ADDR]]) ; CHECK: sh $[[RES]], 0($[[US_ADDR]])
; CHECK: .end orUs1 ; CHECK: .end orUs1
ret void ret void
} }
; Function Attrs: noinline nounwind ; Function Attrs: noinline nounwind
define void @xorUs() #0 { define void @xorUs() #0 {
entry: entry:
▲ Show 20 Lines • Show All 45 Lines • ▼ Show 20 Linesentry:
store i16 %xor, i16* @us, align 2, !tbaa !5 store i16 %xor, i16* @us, align 2, !tbaa !5
; CHECK-LABEL: .ent xorUs1 ; CHECK-LABEL: .ent xorUs1
; CHECK: lui $[[REG_GPa:[0-9]+]], %hi(_gp_disp) ; CHECK: lui $[[REG_GPa:[0-9]+]], %hi(_gp_disp)
; CHECK: addiu $[[REG_GPb:[0-9]+]], $[[REG_GPa]], %lo(_gp_disp) ; CHECK: addiu $[[REG_GPb:[0-9]+]], $[[REG_GPa]], %lo(_gp_disp)
; CHECK: addu $[[REG_GP:[0-9]+]], $[[REG_GPb]], $25 ; CHECK: addu $[[REG_GP:[0-9]+]], $[[REG_GPb]], $25
; CHECK-DAG: lw $[[US_ADDR:[0-9]+]], %got(us)($[[REG_GP]]) ; CHECK-DAG: lw $[[US_ADDR:[0-9]+]], %got(us)($[[REG_GP]])
; CHECK-DAG: lw $[[US1_ADDR:[0-9]+]], %got(us1)($[[REG_GP]]) ; CHECK-DAG: lw $[[US1_ADDR:[0-9]+]], %got(us1)($[[REG_GP]])
; CHECK-DAG: lhu $[[US1:[0-9]+]], 0($[[US1_ADDR]]) ; CHECK-DAG: lhu $[[US1:[0-9]+]], 0($[[US1_ADDR]])
; CHECK-DAG: addiu $[[CONST_Neg5512:[0-9]+]], $zero, -5512 ; CHECK-DAG: ori $[[CONST_60024:[0-9]+]], $zero, 60024
; CHECK-DAG: xor $[[RES:[0-9]+]], $[[US1]], $[[CONST_Neg5512]] ; CHECK-DAG: xor $[[RES:[0-9]+]], $[[US1]], $[[CONST_60024]]
; CHECK: sh $[[RES]], 0($[[US_ADDR]]) ; CHECK: sh $[[RES]], 0($[[US_ADDR]])
; CHECK: .end xorUs1 ; CHECK: .end xorUs1
ret void ret void
} }
attributes #0 = { noinline nounwind "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" } attributes #0 = { noinline nounwind "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
attributes #1 = { nounwind "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" } attributes #1 = { nounwind "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
attributes #2 = { nounwind } attributes #2 = { nounwind }
Show All 11 Lines

llvm/trunk/test/CodeGen/Mips/Fast-ISel/simplestorei.ll

Show All 16 Lines; CHECK: sw $[[REG1]], 0($[[REG2]])
ret void ret void
} }
; Function Attrs: nounwind ; Function Attrs: nounwind
define void @si2_2() #0 { define void @si2_2() #0 {
entry: entry:
store i32 -32768, i32* @ijk, align 4 store i32 -32768, i32* @ijk, align 4
; CHECK: .ent si2_2 ; CHECK: .ent si2_2
; CHECK: addiu $[[REG1:[0-9]+]], $zero, -32768 ; CHECK: lui $[[REG1:[0-9]+]], 65535
; CHECK: lw $[[REG2:[0-9]+]], %got(ijk)(${{[0-9]+}}) ; CHECK: ori $[[REG2:[0-9]+]], $[[REG1]], 32768
; CHECK: sw $[[REG1]], 0($[[REG2]]) ; CHECK: lw $[[REG3:[0-9]+]], %got(ijk)(${{[0-9]+}})
; CHECK: sw $[[REG2]], 0($[[REG3]])
ret void ret void
} }
; Function Attrs: nounwind ; Function Attrs: nounwind
define void @ui2_1() #0 { define void @ui2_1() #0 {
entry: entry:
store i32 65535, i32* @ijk, align 4 store i32 65535, i32* @ijk, align 4
; CHECK: .ent ui2_1 ; CHECK: .ent ui2_1
Show All 32 Lines