From 25ac7178c34b49a2dd9289487ba74f9b530fc9ad Mon Sep 17 00:00:00 2001 From: Daniel Sanders Date: Tue, 13 Jun 2017 23:42:32 +0000 Subject: [PATCH] [globalisel][legalizer] G_LOAD/G_STORE NarrowScalar should not emit G_GEP x, 0. Summary: When legalizing G_LOAD/G_STORE using NarrowScalar, we should avoid emitting %0 = G_CONSTANT ty 0 %1 = G_GEP %x, %0 since it's cheaper to not emit the redundant instructions than it is to fold them away later. Reviewers: qcolombet, t.p.northover, ab, rovka, aditya_nandakumar, kristof.beyls Reviewed By: qcolombet Subscribers: javed.absar, llvm-commits, igorb Differential Revision: https://reviews.llvm.org/D32746 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@305340 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../CodeGen/GlobalISel/MachineIRBuilder.h | 24 +++++++++++++++-- lib/CodeGen/GlobalISel/LegalizerHelper.cpp | 27 ++++++++++--------- lib/CodeGen/GlobalISel/MachineIRBuilder.cpp | 18 +++++++++++++ .../GlobalISel/legalize-load-store.mir | 8 ++---- 4 files changed, 57 insertions(+), 20 deletions(-) diff --git a/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h b/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h index db72f78c832..4e7b8350038 100644 --- a/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h +++ b/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h @@ -40,8 +40,8 @@ class MachineIRBuilder { MachineFunction *MF; /// Information used to access the description of the opcodes. const TargetInstrInfo *TII; - /// Information used to verify types are consistent. - const MachineRegisterInfo *MRI; + /// Information used to verify types are consistent and to create virtual registers. + MachineRegisterInfo *MRI; /// Debug location to be set to any instruction we create. DebugLoc DL; @@ -229,6 +229,26 @@ public: MachineInstrBuilder buildGEP(unsigned Res, unsigned Op0, unsigned Op1); + /// Materialize and insert \p Res = G_GEP \p Op0, (G_CONSTANT \p Value) + /// + /// G_GEP adds \p Value bytes to the pointer specified by \p Op0, + /// storing the resulting pointer in \p Res. If \p Value is zero then no + /// G_GEP or G_CONSTANT will be created and \pre Op0 will be assigned to + /// \p Res. + /// + /// \pre setBasicBlock or setMI must have been called. + /// \pre \p Op0 must be a generic virtual register with pointer type. + /// \pre \p ValueTy must be a scalar type. + /// \pre \p Res must be 0. This is to detect confusion between + /// materializeGEP() and buildGEP(). + /// \post \p Res will either be a new generic virtual register of the same + /// type as \p Op0 or \p Op0 itself. + /// + /// \return a MachineInstrBuilder for the newly created instruction. + Optional materializeGEP(unsigned &Res, unsigned Op0, + const LLT &ValueTy, + uint64_t Value); + /// Build and insert \p Res = G_PTR_MASK \p Op0, \p NumBits /// /// G_PTR_MASK clears the low bits of a pointer operand without destroying its diff --git a/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/lib/CodeGen/GlobalISel/LegalizerHelper.cpp index ef5818dabe2..e05b355254c 100644 --- a/lib/CodeGen/GlobalISel/LegalizerHelper.cpp +++ b/lib/CodeGen/GlobalISel/LegalizerHelper.cpp @@ -237,17 +237,18 @@ LegalizerHelper::LegalizeResult LegalizerHelper::narrowScalar(MachineInstr &MI, unsigned NarrowSize = NarrowTy.getSizeInBits(); int NumParts = MRI.getType(MI.getOperand(0).getReg()).getSizeInBits() / NarrowSize; - LLT NarrowPtrTy = LLT::pointer( - MRI.getType(MI.getOperand(1).getReg()).getAddressSpace(), NarrowSize); + LLT OffsetTy = LLT::scalar( + MRI.getType(MI.getOperand(1).getReg()).getScalarSizeInBits()); SmallVector DstRegs; for (int i = 0; i < NumParts; ++i) { unsigned DstReg = MRI.createGenericVirtualRegister(NarrowTy); - unsigned SrcReg = MRI.createGenericVirtualRegister(NarrowPtrTy); - unsigned Offset = MRI.createGenericVirtualRegister(LLT::scalar(64)); + unsigned SrcReg = 0; + unsigned Adjustment = i * NarrowSize / 8; + + MIRBuilder.materializeGEP(SrcReg, MI.getOperand(1).getReg(), OffsetTy, + Adjustment); - MIRBuilder.buildConstant(Offset, i * NarrowSize / 8); - MIRBuilder.buildGEP(SrcReg, MI.getOperand(1).getReg(), Offset); // TODO: This is conservatively correct, but we probably want to split the // memory operands in the future. MIRBuilder.buildLoad(DstReg, SrcReg, **MI.memoperands_begin()); @@ -263,17 +264,19 @@ LegalizerHelper::LegalizeResult LegalizerHelper::narrowScalar(MachineInstr &MI, unsigned NarrowSize = NarrowTy.getSizeInBits(); int NumParts = MRI.getType(MI.getOperand(0).getReg()).getSizeInBits() / NarrowSize; - LLT NarrowPtrTy = LLT::pointer( - MRI.getType(MI.getOperand(1).getReg()).getAddressSpace(), NarrowSize); + LLT OffsetTy = LLT::scalar( + MRI.getType(MI.getOperand(1).getReg()).getScalarSizeInBits()); SmallVector SrcRegs; extractParts(MI.getOperand(0).getReg(), NarrowTy, NumParts, SrcRegs); for (int i = 0; i < NumParts; ++i) { - unsigned DstReg = MRI.createGenericVirtualRegister(NarrowPtrTy); - unsigned Offset = MRI.createGenericVirtualRegister(LLT::scalar(64)); - MIRBuilder.buildConstant(Offset, i * NarrowSize / 8); - MIRBuilder.buildGEP(DstReg, MI.getOperand(1).getReg(), Offset); + unsigned DstReg = 0; + unsigned Adjustment = i * NarrowSize / 8; + + MIRBuilder.materializeGEP(DstReg, MI.getOperand(1).getReg(), OffsetTy, + Adjustment); + // TODO: This is conservatively correct, but we probably want to split the // memory operands in the future. MIRBuilder.buildStore(SrcRegs[i], DstReg, **MI.memoperands_begin()); diff --git a/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp b/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp index 54ef7e5c5a1..79d312fb52c 100644 --- a/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp +++ b/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp @@ -191,6 +191,24 @@ MachineInstrBuilder MachineIRBuilder::buildGEP(unsigned Res, unsigned Op0, .addUse(Op1); } +Optional +MachineIRBuilder::materializeGEP(unsigned &Res, unsigned Op0, + const LLT &ValueTy, uint64_t Value) { + assert(Res == 0 && "Res is a result argument"); + assert(ValueTy.isScalar() && "invalid offset type"); + + if (Value == 0) { + Res = Op0; + return None; + } + + Res = MRI->createGenericVirtualRegister(MRI->getType(Op0)); + unsigned TmpReg = MRI->createGenericVirtualRegister(ValueTy); + + buildConstant(TmpReg, Value); + return buildGEP(Res, Op0, TmpReg); +} + MachineInstrBuilder MachineIRBuilder::buildPtrMask(unsigned Res, unsigned Op0, uint32_t NumBits) { assert(MRI->getType(Res).isPointer() && diff --git a/test/CodeGen/AArch64/GlobalISel/legalize-load-store.mir b/test/CodeGen/AArch64/GlobalISel/legalize-load-store.mir index c806b4a7060..ce913d211ae 100644 --- a/test/CodeGen/AArch64/GlobalISel/legalize-load-store.mir +++ b/test/CodeGen/AArch64/GlobalISel/legalize-load-store.mir @@ -53,9 +53,7 @@ body: | ; CHECK: %7(<2 x s32>) = G_LOAD %0(p0) :: (load 8 from %ir.addr) %7(<2 x s32>) = G_LOAD %0(p0) :: (load 8 from %ir.addr) - ; CHECK: [[OFFSET0:%[0-9]+]](s64) = G_CONSTANT i64 0 - ; CHECK: [[GEP0:%[0-9]+]](p0) = G_GEP %0, [[OFFSET0]](s64) - ; CHECK: [[LOAD0:%[0-9]+]](s64) = G_LOAD [[GEP0]](p0) :: (load 16 from %ir.addr) + ; CHECK: [[LOAD0:%[0-9]+]](s64) = G_LOAD %0(p0) :: (load 16 from %ir.addr) ; CHECK: [[OFFSET1:%[0-9]+]](s64) = G_CONSTANT i64 8 ; CHECK: [[GEP1:%[0-9]+]](p0) = G_GEP %0, [[OFFSET1]](s64) ; CHECK: [[LOAD1:%[0-9]+]](s64) = G_LOAD [[GEP1]](p0) :: (load 16 from %ir.addr) @@ -105,9 +103,7 @@ body: | ; CHECK: G_STORE %0(p0), %0(p0) :: (store 8 into %ir.addr) G_STORE %0(p0), %0(p0) :: (store 8 into %ir.addr) - ; CHECK: [[OFFSET0:%[0-9]+]](s64) = G_CONSTANT i64 0 - ; CHECK: [[GEP0:%[0-9]+]](p0) = G_GEP %0, [[OFFSET0]](s64) - ; CHECK: G_STORE %5(s64), [[GEP0]](p0) :: (store 16 into %ir.addr) + ; CHECK: G_STORE %5(s64), %0(p0) :: (store 16 into %ir.addr) ; CHECK: [[OFFSET1:%[0-9]+]](s64) = G_CONSTANT i64 8 ; CHECK: [[GEP1:%[0-9]+]](p0) = G_GEP %0, [[OFFSET1]](s64) ; CHECK: G_STORE %6(s64), [[GEP1]](p0) :: (store 16 into %ir.addr) -- 2.50.1