[SelectionDAG] set dereferenceable flag when expanding memcpy/memmove

[inouehrs]

Summary

When SelectionDAG expands memcpy (or memmove) call into a sequence of load and store instructions, it disregards dereferenceable flag even the source pointer is known to be dereferenceable.
This results in an assertion failure if SelectionDAG commonizes a load instruction generated for memcpy with another load instruction for the source pointer.
This patch makes SelectionDAG to set the dereferenceable flag for the load instructions properly to avoid the assertion failure.

[hfinkel]

lib/CodeGen/MachineInstr.cpp

576

Creating temporary instructions in helper functions is something we try very hard not to do. Please don't do it here.

The underlying isDereferenceableAndAlignedPointer implementation function in lib/Analysis/Loads.cpp actually takes a size. Feel free to expose an interface directly to that functionality if necessary.

[inouehrs]

updated the implementation to avoid creating temporary instructions

[inouehrs]

lib/CodeGen/MachineInstr.cpp

576

I added an interface to pass size as a parameter in isDereferenceableAndAlignedPointer and eliminated the temporary instructions in the helper.
Thank you for the suggestion.

[inouehrs]

• added a test case extracted from a large application which causes an assertion failure

[inouehrs]

minor touch up in the test case

[hfinkel]

Please rebase after r306193 and post the patch with full context.

[inouehrs]

• rebased after https://reviews.llvm.org/rL306193

[hfinkel]

LGTM

include/llvm/Analysis/Loads.h

                                         const Instruction *CtxI = nullptr,
                                         const DominatorTree *DT = nullptr);
 
+/// Returns true if V is always dereferenceable for Size byte with alignment
+/// greater or equal than requested. If the context instruction is specified
+/// performs context-sensitive analysis and returns true if the pointer is
+/// dereferenceable at the specified instruction.
+bool isDereferenceableAndAlignedPointer(const Value *V, unsigned Align,
+                                        const APInt &Size,
+                                        const DataLayout &DL,
+                                        const Instruction *CtxI = nullptr,
+                                        const DominatorTree *DT = nullptr);
+
 /// Return true if we know that executing a load from this value cannot trap.
 ///
 /// If DT and ScanFrom are specified this method performs context-sensitive

include/llvm/CodeGen/MachineMemOperand.h

     return MachinePointerInfo(V.get<const PseudoSourceValue*>(), Offset+O);
   }
 
+  /// Return true if memory region [V+Offset, V+Offset+Size) is
+  /// known to be dereferenceable.
+  bool isDereferenceable(unsigned Size, LLVMContext &C,
+                         const DataLayout &DL) const;
+
   /// Return the LLVM IR address space number that this pointer points into.
   unsigned getAddrSpace() const;
 

lib/Analysis/Loads.cpp

 }
 
 bool llvm::isDereferenceableAndAlignedPointer(const Value *V, unsigned Align,
+                                              const APInt &Size,
+                                              const DataLayout &DL,
+                                              const Instruction *CtxI,
+                                              const DominatorTree *DT) {
+  SmallPtrSet<const Value *, 32> Visited;
+  return ::isDereferenceableAndAlignedPointer(V, Align, Size, DL, CtxI, DT,
+                                              Visited);
+}
+
+bool llvm::isDereferenceableAndAlignedPointer(const Value *V, unsigned Align,
                                               const DataLayout &DL,
                                               const Instruction *CtxI,
                                               const DominatorTree *DT) {

   if (!Ty->isSized())
     return false;
 
-  SmallPtrSet<const Value *, 32> Visited;
-  return ::isDereferenceableAndAlignedPointer(
+  return isDereferenceableAndAlignedPointer(
       V, Align, APInt(DL.getTypeSizeInBits(VTy), DL.getTypeStoreSize(Ty)), DL,
-      CtxI, DT, Visited);
+      CtxI, DT);
 }
 
 bool llvm::isDereferenceablePointer(const Value *V, const DataLayout &DL,

lib/CodeGen/MachineInstr.cpp

 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Analysis/AliasAnalysis.h"
+#include "llvm/Analysis/Loads.h"
 #include "llvm/Analysis/MemoryLocation.h"
 #include "llvm/CodeGen/GlobalISel/RegisterBank.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"

   return cast<PointerType>(V.get<const Value*>()->getType())->getAddressSpace();
 }
 
+/// isDereferenceable - Return true if V is always dereferenceable for 
+/// Offset + Size byte.
+bool MachinePointerInfo::isDereferenceable(unsigned Size, LLVMContext &C,
+                                           const DataLayout &DL) const {
+  if (!V.is<const Value*>())
+    return false;
+
+  const Value *BasePtr = V.get<const Value*>();
+  if (BasePtr == nullptr)
+    return false;
+
+  return isDereferenceableAndAlignedPointer(BasePtr, 1,
+                                            APInt(DL.getPointerSize(),
+                                                  Offset + Size),
+                                            DL);
+}
+
 /// getConstantPool - Return a MachinePointerInfo record that refers to the
 /// constant pool.
 MachinePointerInfo MachinePointerInfo::getConstantPool(MachineFunction &MF) {

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

   // TODO: In the AlwaysInline case, if the size is big then generate a loop
   // rather than maybe a humongous number of loads and stores.
   const TargetLowering &TLI = DAG.getTargetLoweringInfo();
+  const DataLayout &DL = DAG.getDataLayout();
+  LLVMContext &C = *DAG.getContext();
   std::vector<EVT> MemOps;
   bool DstAlignCanChange = false;
   MachineFunction &MF = DAG.getMachineFunction();

     return SDValue();
 
   if (DstAlignCanChange) {
-    Type *Ty = MemOps[0].getTypeForEVT(*DAG.getContext());
-    unsigned NewAlign = (unsigned)DAG.getDataLayout().getABITypeAlignment(Ty);
+    Type *Ty = MemOps[0].getTypeForEVT(C);
+    unsigned NewAlign = (unsigned)DL.getABITypeAlignment(Ty);
 
     // Don't promote to an alignment that would require dynamic stack
     // realignment.
     const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
     if (!TRI->needsStackRealignment(MF))
       while (NewAlign > Align &&
-             DAG.getDataLayout().exceedsNaturalStackAlignment(NewAlign))
+             DL.exceedsNaturalStackAlignment(NewAlign))
           NewAlign /= 2;
 
     if (NewAlign > Align) {

       // thing to do is generate a LoadExt/StoreTrunc pair.  These simplify
       // to Load/Store if NVT==VT.
       // FIXME does the case above also need this?
-      EVT NVT = TLI.getTypeToTransformTo(*DAG.getContext(), VT);
+      EVT NVT = TLI.getTypeToTransformTo(C, VT);
       assert(NVT.bitsGE(VT));
+
+      bool isDereferenceable =
+        SrcPtrInfo.getWithOffset(SrcOff).isDereferenceable(VTSize, C, DL);
+      MachineMemOperand::Flags SrcMMOFlags = MMOFlags;
+      if (isDereferenceable)
+        SrcMMOFlags |= MachineMemOperand::MODereferenceable;
+
       Value = DAG.getExtLoad(ISD::EXTLOAD, dl, NVT, Chain,
                              DAG.getMemBasePlusOffset(Src, SrcOff, dl),
                              SrcPtrInfo.getWithOffset(SrcOff), VT,
-                             MinAlign(SrcAlign, SrcOff), MMOFlags);
+                             MinAlign(SrcAlign, SrcOff), SrcMMOFlags);
       OutChains.push_back(Value.getValue(1));
       Store = DAG.getTruncStore(
           Chain, dl, Value, DAG.getMemBasePlusOffset(Dst, DstOff, dl),

   // Expand memmove to a series of load and store ops if the size operand falls
   // below a certain threshold.
   const TargetLowering &TLI = DAG.getTargetLoweringInfo();
+  const DataLayout &DL = DAG.getDataLayout();
+  LLVMContext &C = *DAG.getContext();
   std::vector<EVT> MemOps;
   bool DstAlignCanChange = false;
   MachineFunction &MF = DAG.getMachineFunction();

     return SDValue();
 
   if (DstAlignCanChange) {
-    Type *Ty = MemOps[0].getTypeForEVT(*DAG.getContext());
-    unsigned NewAlign = (unsigned)DAG.getDataLayout().getABITypeAlignment(Ty);
+    Type *Ty = MemOps[0].getTypeForEVT(C);
+    unsigned NewAlign = (unsigned)DL.getABITypeAlignment(Ty);
     if (NewAlign > Align) {
       // Give the stack frame object a larger alignment if needed.
       if (MFI.getObjectAlignment(FI->getIndex()) < NewAlign)

     unsigned VTSize = VT.getSizeInBits() / 8;
     SDValue Value;
 
+    bool isDereferenceable =
+      SrcPtrInfo.getWithOffset(SrcOff).isDereferenceable(VTSize, C, DL);
+    MachineMemOperand::Flags SrcMMOFlags = MMOFlags;
+    if (isDereferenceable)
+      SrcMMOFlags |= MachineMemOperand::MODereferenceable;
+
     Value =
         DAG.getLoad(VT, dl, Chain, DAG.getMemBasePlusOffset(Src, SrcOff, dl),
-                    SrcPtrInfo.getWithOffset(SrcOff), SrcAlign, MMOFlags);
+                    SrcPtrInfo.getWithOffset(SrcOff), SrcAlign, SrcMMOFlags);
     LoadValues.push_back(Value);
     LoadChains.push_back(Value.getValue(1));
     SrcOff += VTSize;

test/CodeGen/PowerPC/memcpy_dereferenceable.ll

+; RUN: llc -verify-machineinstrs -mcpu=pwr8 -mtriple=powerpc64le-unknown-linux-gnu < %s | FileCheck %s
+
+; This code causes an assertion failure if dereferenceable flag is not properly set in the load generated for memcpy
+
+; CHECK-LABEL: @func
+; CHECK: lxvd2x [[VREG:[0-9]+]], {{[0-9]+}}, {{[0-9]+}}
+; CHECK-NOT: lxvd2x
+; CHECK: stxvd2x [[VREG:[0-9]+]], {{[0-9]+}}, {{[0-9]+}}
+; CHECK: stxvd2x [[VREG:[0-9]+]], {{[0-9]+}}, {{[0-9]+}}
+; CHECK: blr
+
+define void @func(i1 %flag) {
+entry:
+  %pairs = alloca [4 x [2 x i64]], align 8
+  %pair1 = getelementptr inbounds [4 x [2 x i64]], [4 x [2 x i64]]* %pairs, i64 0, i64 1
+  %pair2 = getelementptr inbounds [4 x [2 x i64]], [4 x [2 x i64]]* %pairs, i64 0, i64 2
+  %pvec1 = bitcast [2 x i64]* %pair1 to <2 x i64>*
+  %pvec2 = bitcast [2 x i64]* %pair2 to <2 x i64>*
+  %dst = bitcast [4 x [2 x i64]]* %pairs to i8*
+  %src = bitcast [2 x i64]* %pair2 to i8*
+  br i1 %flag, label %end, label %dummy
+
+end:
+  ; copy third element into first element by memcpy
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* nonnull %dst, i8* %src, i64 16, i32 8, i1 false)
+  ; copy third element into second element by LD/ST
+  %vec2 = load <2 x i64>, <2 x i64>* %pvec2, align 8
+  store <2 x i64> %vec2, <2 x i64>* %pvec1, align 8
+  ret void
+
+dummy:
+  ; to make use of %src in another BB
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %src, i8* %src, i64 0, i32 0, i1 false)
+  br label %end
+}
+
+
+; CHECK-LABEL: @func2
+; CHECK: lxvd2x [[VREG:[0-9]+]], {{[0-9]+}}, {{[0-9]+}}
+; CHECK-NOT: lxvd2x
+; CHECK: stxvd2x [[VREG:[0-9]+]], {{[0-9]+}}, {{[0-9]+}}
+; CHECK: stxvd2x [[VREG:[0-9]+]], {{[0-9]+}}, {{[0-9]+}}
+; CHECK: blr
+
+define void @func2(i1 %flag) {
+entry:
+  %pairs = alloca [4 x [2 x i64]], align 8
+  %pair1 = getelementptr inbounds [4 x [2 x i64]], [4 x [2 x i64]]* %pairs, i64 0, i64 1
+  %pair2 = getelementptr inbounds [4 x [2 x i64]], [4 x [2 x i64]]* %pairs, i64 0, i64 2
+  %pvec1 = bitcast [2 x i64]* %pair1 to <2 x i64>*
+  %pvec2 = bitcast [2 x i64]* %pair2 to <2 x i64>*
+  %dst = bitcast [4 x [2 x i64]]* %pairs to i8*
+  %src = bitcast [2 x i64]* %pair2 to i8*
+  br i1 %flag, label %end, label %dummy
+
+end:
+  ; copy third element into first element by memcpy
+  call void @llvm.memmove.p0i8.p0i8.i64(i8* nonnull %dst, i8* %src, i64 16, i32 8, i1 false)
+  ; copy third element into second element by LD/ST
+  %vec2 = load <2 x i64>, <2 x i64>* %pvec2, align 8
+  store <2 x i64> %vec2, <2 x i64>* %pvec1, align 8
+  ret void
+
+dummy:
+  ; to make use of %src in another BB
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %src, i8* %src, i64 0, i32 0, i1 false)
+  br label %end
+}
+
+; Function Attrs: argmemonly nounwind
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i32, i1) #1
+declare void @llvm.memmove.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i32, i1) #1
+
+attributes #1 = { argmemonly nounwind }