[ScheduleDAGInstrs] fix behavior of getUnderlyingObjectsForCodeGen when no identifiable object found

[inouehrs]

Summary

This patch fixes the bug introduced in my previous patch https://reviews.llvm.org/D35907 reported by http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20171002/491452.html .

Before D35907, when GetUnderlyingObjects fails to find an identifiable object, allMMOsOkay lambda in getUnderlyingObjectsForInstr returns false and Objects vector is cleared. This behavior is unintentionally changed by D35907.

This patch makes the behavior for such case same as the previous behavior.
Since D35907 introduced a wrapper function getUnderlyingObjectsForCodeGen around GetUnderlyingObjects, getUnderlyingObjectsForCodeGen is modified to return a boolean value to ask the caller to clear the Objects vector.

[MatzeB]

This looks good, but makes me wonder if we shouldn't do the same change (returning a bool instead of looking at the vector containing 0 or more elements) to getUnderlyingObjectsForInstr() for consistency. Could you try doing that?

[inouehrs]

[[ https://github.com/llvm-mirror/llvm/blob/c19eec32627989457c78dbfd91f79c3be1b99422/lib/CodeGen/ScheduleDAGInstrs.cpp#L187-L223 | In the old implementation of getUnderlyingObjectsForInstr ]], Objects vector is cleared when getUnderlyingObjects (and hence GetUnderlyingObjects) returns an unidentified object, but not cleared when getUnderlyingObjects returns no object.
I added boolean return value in getUnderlyingObjectsForCodeGen to distinguish these two cases, since it returns empty vector for both cases.

Do you mean using boolean return value also in getUnderlyingObjectsForInstr for consistency?

[MatzeB]

In D38735#894467, @inouehrs wrote:

Do you mean using boolean return value also in getUnderlyingObjectsForInstr for consistency?

Yes that's what I meant.

[inouehrs]

@MatzeB Thank you for the clarification. How about this?

[MatzeB]

Thanks, LGTM

include/llvm/Analysis/ValueTracking.h

class Value;
   /// should not assume that Curr and Prev share the same underlying object thus
   /// it shouldn't look through the phi above.
   void GetUnderlyingObjects(Value *V, SmallVectorImpl<Value *> &Objects,
                             const DataLayout &DL, LoopInfo *LI = nullptr,
                             unsigned MaxLookup = 6);
 
   /// This is a wrapper around GetUnderlyingObjects and adds support for basic
   /// ptrtoint+arithmetic+inttoptr sequences.
-  void getUnderlyingObjectsForCodeGen(const Value *V,
+  bool getUnderlyingObjectsForCodeGen(const Value *V,
                             SmallVectorImpl<Value *> &Objects,
                             const DataLayout &DL);
 
   /// Return true if the only users of this pointer are lifetime markers.
   bool onlyUsedByLifetimeMarkers(const Value *V);
 
   /// Return true if the instruction does not have any effects besides
   /// calculating the result and does not have undefined behavior.

lib/Analysis/ValueTracking.cpp

if (const Operator *U = dyn_cast<Operator>(V)) {
       return V;
     }
     assert(V->getType()->isIntegerTy() && "Unexpected operand type!");
   } while (true);
 }
 
 /// This is a wrapper around GetUnderlyingObjects and adds support for basic
 /// ptrtoint+arithmetic+inttoptr sequences.
-void llvm::getUnderlyingObjectsForCodeGen(const Value *V,
+/// It returns false if unidentified object is found in GetUnderlyingObjects.
+bool llvm::getUnderlyingObjectsForCodeGen(const Value *V,
                           SmallVectorImpl<Value *> &Objects,
                           const DataLayout &DL) {
   SmallPtrSet<const Value *, 16> Visited;
   SmallVector<const Value *, 4> Working(1, V);
   do {
     V = Working.pop_back_val();
 
     SmallVector<Value *, 4> Objs;
for (Value *V : Objs) {
           Working.push_back(O);
           continue;
         }
       }
       // If GetUnderlyingObjects fails to find an identifiable object,
       // getUnderlyingObjectsForCodeGen also fails for safety.
       if (!isIdentifiedObject(V)) {
         Objects.clear();
-        return;
+        return false;
       }
       Objects.push_back(const_cast<Value *>(V));
     }
   } while (!Working.empty());
+  return true;
 }
 
 /// Return true if the only users of this pointer are lifetime markers.
 bool llvm::onlyUsedByLifetimeMarkers(const Value *V) {
   for (const User *U : V->users()) {
     const IntrinsicInst *II = dyn_cast<IntrinsicInst>(U);
     if (!II) return false;
 

lib/CodeGen/ScheduleDAGInstrs.cpp

for (const MachineMemOperand *MMO : MI->memoperands()) {
         // such aliases.
         if (PSV->isAliased(&MFI))
           return false;
 
         bool MayAlias = PSV->mayAlias(&MFI);
         Objects.push_back(UnderlyingObjectsVector::value_type(PSV, MayAlias));
       } else if (const Value *V = MMO->getValue()) {
         SmallVector<Value *, 4> Objs;
-        getUnderlyingObjectsForCodeGen(V, Objs, DL);
+        if (!getUnderlyingObjectsForCodeGen(V, Objs, DL))
+          return false;
 
         for (Value *V : Objs) {
           assert(isIdentifiedObject(V));
           Objects.push_back(UnderlyingObjectsVector::value_type(V, true));
         }
       } else
         return false;
     }