[llvm] r305340 - [globalisel][legalizer] G_LOAD/G_STORE NarrowScalar should not emit G_GEP x, 0.

Daniel Sanders via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 13 16:42:33 PDT 2017


Author: dsanders
Date: Tue Jun 13 18:42:32 2017
New Revision: 305340

URL: http://llvm.org/viewvc/llvm-project?rev=305340&view=rev
Log:
[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

Modified:
    llvm/trunk/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
    llvm/trunk/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
    llvm/trunk/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
    llvm/trunk/test/CodeGen/AArch64/GlobalISel/legalize-load-store.mir

Modified: llvm/trunk/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h?rev=305340&r1=305339&r2=305340&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h (original)
+++ llvm/trunk/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h Tue Jun 13 18:42:32 2017
@@ -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<def> = 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<MachineInstrBuilder> materializeGEP(unsigned &Res, unsigned Op0,
+                                               const LLT &ValueTy,
+                                               uint64_t Value);
+
   /// Build and insert \p Res<def> = G_PTR_MASK \p Op0, \p NumBits
   ///
   /// G_PTR_MASK clears the low bits of a pointer operand without destroying its

Modified: llvm/trunk/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/LegalizerHelper.cpp?rev=305340&r1=305339&r2=305340&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/GlobalISel/LegalizerHelper.cpp (original)
+++ llvm/trunk/lib/CodeGen/GlobalISel/LegalizerHelper.cpp Tue Jun 13 18:42:32 2017
@@ -237,17 +237,18 @@ LegalizerHelper::LegalizeResult Legalize
     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<unsigned, 2> 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 Legalize
     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<unsigned, 2> 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());

Modified: llvm/trunk/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp?rev=305340&r1=305339&r2=305340&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp (original)
+++ llvm/trunk/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp Tue Jun 13 18:42:32 2017
@@ -191,6 +191,24 @@ MachineInstrBuilder MachineIRBuilder::bu
       .addUse(Op1);
 }
 
+Optional<MachineInstrBuilder>
+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() &&

Modified: llvm/trunk/test/CodeGen/AArch64/GlobalISel/legalize-load-store.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/GlobalISel/legalize-load-store.mir?rev=305340&r1=305339&r2=305340&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/GlobalISel/legalize-load-store.mir (original)
+++ llvm/trunk/test/CodeGen/AArch64/GlobalISel/legalize-load-store.mir Tue Jun 13 18:42:32 2017
@@ -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)




More information about the llvm-commits mailing list