[llvm] r314928 - Recommit r314561 after fixing msan build failure

Xinliang David Li via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 4 17:28:20 PDT 2017


I did a self build with thinLTO (using a debug compiler), but could not
reproduce the problem.  What is exact build configuration used?

thanks,

David

On Wed, Oct 4, 2017 at 3:22 PM, Xinliang David Li <davidxl at google.com>
wrote:

> Looking ..
>
> David
>
> On Wed, Oct 4, 2017 at 3:19 PM, Juergen Ributzka <juergen at ributzka.de>
> wrote:
>
>> Hi David,
>>
>> I think this broke the Green Dragon ThinLTO Bot:
>> http://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/5938/
>>
>> Could you please take a look?
>>
>> Thanks
>>
>> Cheers,
>> Juergen
>>
>> On Wed, Oct 4, 2017 at 1:17 PM, Xinliang David Li via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> Author: davidxl
>>> Date: Wed Oct  4 13:17:55 2017
>>> New Revision: 314928
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=314928&view=rev
>>> Log:
>>> Recommit r314561 after fixing msan build failure
>>>
>>> (trial 2) Incoming val defined by terminator instruction which
>>> also requires bitcasts can not be handled.
>>>
>>>
>>> Added:
>>>     llvm/trunk/test/Transforms/InstCombine/intptr1.ll
>>>       - copied unchanged from r314621, llvm/trunk/test/Transforms/Ins
>>> tCombine/intptr1.ll
>>>     llvm/trunk/test/Transforms/InstCombine/intptr2.ll
>>>       - copied unchanged from r314621, llvm/trunk/test/Transforms/Ins
>>> tCombine/intptr2.ll
>>>     llvm/trunk/test/Transforms/InstCombine/intptr3.ll
>>>       - copied unchanged from r314621, llvm/trunk/test/Transforms/Ins
>>> tCombine/intptr3.ll
>>>     llvm/trunk/test/Transforms/InstCombine/intptr4.ll
>>>       - copied unchanged from r314621, llvm/trunk/test/Transforms/Ins
>>> tCombine/intptr4.ll
>>> Modified:
>>>     llvm/trunk/lib/Transforms/InstCombine/InstCombineInternal.h
>>>     llvm/trunk/lib/Transforms/InstCombine/InstCombinePHI.cpp
>>>
>>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineInternal.h
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transform
>>> s/InstCombine/InstCombineInternal.h?rev=314928&r1=314927&r2=
>>> 314928&view=diff
>>> ============================================================
>>> ==================
>>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineInternal.h
>>> (original)
>>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineInternal.h Wed
>>> Oct  4 13:17:55 2017
>>> @@ -670,6 +670,10 @@ private:
>>>    Instruction *FoldPHIArgGEPIntoPHI(PHINode &PN);
>>>    Instruction *FoldPHIArgLoadIntoPHI(PHINode &PN);
>>>    Instruction *FoldPHIArgZextsIntoPHI(PHINode &PN);
>>> +  /// If an integer typed PHI has only one use which is an IntToPtr
>>> operation,
>>> +  /// replace the PHI with an existing pointer typed PHI if it exists.
>>> Otherwise
>>> +  /// insert a new pointer typed PHI and replace the original one.
>>> +  Instruction *FoldIntegerTypedPHI(PHINode &PN);
>>>
>>>    /// Helper function for FoldPHIArgXIntoPHI() to set debug location
>>> for the
>>>    /// folded operation.
>>>
>>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombinePHI.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transform
>>> s/InstCombine/InstCombinePHI.cpp?rev=314928&r1=314927&r2=314
>>> 928&view=diff
>>> ============================================================
>>> ==================
>>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombinePHI.cpp (original)
>>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombinePHI.cpp Wed Oct  4
>>> 13:17:55 2017
>>> @@ -40,6 +40,233 @@ void InstCombiner::PHIArgMergedDebugLoc(
>>>    }
>>>  }
>>>
>>> +// Replace Integer typed PHI PN if the PHI's value is used as a pointer
>>> value.
>>> +// If there is an existing pointer typed PHI that produces the same
>>> value as PN,
>>> +// replace PN and the IntToPtr operation with it. Otherwise, synthesize
>>> a new
>>> +// PHI node:
>>> +//
>>> +// Case-1:
>>> +// bb1:
>>> +//     int_init = PtrToInt(ptr_init)
>>> +//     br label %bb2
>>> +// bb2:
>>> +//    int_val = PHI([int_init, %bb1], [int_val_inc, %bb2]
>>> +//    ptr_val = PHI([ptr_init, %bb1], [ptr_val_inc, %bb2]
>>> +//    ptr_val2 = IntToPtr(int_val)
>>> +//    ...
>>> +//    use(ptr_val2)
>>> +//    ptr_val_inc = ...
>>> +//    inc_val_inc = PtrToInt(ptr_val_inc)
>>> +//
>>> +// ==>
>>> +// bb1:
>>> +//     br label %bb2
>>> +// bb2:
>>> +//    ptr_val = PHI([ptr_init, %bb1], [ptr_val_inc, %bb2]
>>> +//    ...
>>> +//    use(ptr_val)
>>> +//    ptr_val_inc = ...
>>> +//
>>> +// Case-2:
>>> +// bb1:
>>> +//    int_ptr = BitCast(ptr_ptr)
>>> +//    int_init = Load(int_ptr)
>>> +//    br label %bb2
>>> +// bb2:
>>> +//    int_val = PHI([int_init, %bb1], [int_val_inc, %bb2]
>>> +//    ptr_val2 = IntToPtr(int_val)
>>> +//    ...
>>> +//    use(ptr_val2)
>>> +//    ptr_val_inc = ...
>>> +//    inc_val_inc = PtrToInt(ptr_val_inc)
>>> +// ==>
>>> +// bb1:
>>> +//    ptr_init = Load(ptr_ptr)
>>> +//    br label %bb2
>>> +// bb2:
>>> +//    ptr_val = PHI([ptr_init, %bb1], [ptr_val_inc, %bb2]
>>> +//    ...
>>> +//    use(ptr_val)
>>> +//    ptr_val_inc = ...
>>> +//    ...
>>> +//
>>> +Instruction *InstCombiner::FoldIntegerTypedPHI(PHINode &PN) {
>>> +  if (!PN.getType()->isIntegerTy())
>>> +    return nullptr;
>>> +  if (!PN.hasOneUse())
>>> +    return nullptr;
>>> +
>>> +  auto *IntToPtr = dyn_cast<IntToPtrInst>(PN.user_back());
>>> +  if (!IntToPtr)
>>> +    return nullptr;
>>> +
>>> +  // Check if the pointer is actually used as pointer:
>>> +  auto HasPointerUse = [](Instruction *IIP) {
>>> +    for (User *U : IIP->users()) {
>>> +      Value *Ptr = nullptr;
>>> +      if (LoadInst *LoadI = dyn_cast<LoadInst>(U)) {
>>> +        Ptr = LoadI->getPointerOperand();
>>> +      } else if (StoreInst *SI = dyn_cast<StoreInst>(U)) {
>>> +        Ptr = SI->getPointerOperand();
>>> +      } else if (GetElementPtrInst *GI = dyn_cast<GetElementPtrInst>(U))
>>> {
>>> +        Ptr = GI->getPointerOperand();
>>> +      }
>>> +
>>> +      if (Ptr && Ptr == IIP)
>>> +        return true;
>>> +    }
>>> +    return false;
>>> +  };
>>> +
>>> +  if (!HasPointerUse(IntToPtr))
>>> +    return nullptr;
>>> +
>>> +  if (DL.getPointerSizeInBits(IntToPtr->getAddressSpace()) !=
>>> +      DL.getTypeSizeInBits(IntToPtr->getOperand(0)->getType()))
>>> +    return nullptr;
>>> +
>>> +  SmallVector<Value *, 4> AvailablePtrVals;
>>> +  for (unsigned i = 0; i != PN.getNumIncomingValues(); ++i) {
>>> +    Value *Arg = PN.getIncomingValue(i);
>>> +
>>> +    // First look backward:
>>> +    if (auto *PI = dyn_cast<PtrToIntInst>(Arg)) {
>>> +      AvailablePtrVals.emplace_back(PI->getOperand(0));
>>> +      continue;
>>> +    }
>>> +
>>> +    // Next look forward:
>>> +    Value *ArgIntToPtr = nullptr;
>>> +    for (User *U : Arg->users()) {
>>> +      if (isa<IntToPtrInst>(U) && U->getType() == IntToPtr->getType() &&
>>> +          (DT.dominates(cast<Instruction>(U), PN.getIncomingBlock(i))
>>> ||
>>> +           cast<Instruction>(U)->getParent() ==
>>> PN.getIncomingBlock(i))) {
>>> +        ArgIntToPtr = U;
>>> +        break;
>>> +      }
>>> +    }
>>> +
>>> +    if (ArgIntToPtr) {
>>> +      AvailablePtrVals.emplace_back(ArgIntToPtr);
>>> +      continue;
>>> +    }
>>> +
>>> +    // If Arg is defined by a PHI, allow it. This will also create
>>> +    // more opportunities iteratively.
>>> +    if (isa<PHINode>(Arg)) {
>>> +      AvailablePtrVals.emplace_back(Arg);
>>> +      continue;
>>> +    }
>>> +
>>> +    // For a single use integer load:
>>> +    auto *LoadI = dyn_cast<LoadInst>(Arg);
>>> +    if (!LoadI)
>>> +      return nullptr;
>>> +
>>> +    if (!LoadI->hasOneUse())
>>> +      return nullptr;
>>> +
>>> +    // Push the integer typed Load instruction into the available
>>> +    // value set, and fix it up later when the pointer typed PHI
>>> +    // is synthesized.
>>> +    AvailablePtrVals.emplace_back(LoadI);
>>> +  }
>>> +
>>> +  // Now search for a matching PHI
>>> +  auto *BB = PN.getParent();
>>> +  assert(AvailablePtrVals.size() == PN.getNumIncomingValues() &&
>>> +         "Not enough available ptr typed incoming values");
>>> +  PHINode *MatchingPtrPHI = nullptr;
>>> +  for (auto II = BB->begin(), EI = BasicBlock::iterator(BB->getFi
>>> rstNonPHI());
>>> +       II != EI; II++) {
>>> +    PHINode *PtrPHI = dyn_cast<PHINode>(II);
>>> +    if (!PtrPHI || PtrPHI == &PN || PtrPHI->getType() !=
>>> IntToPtr->getType())
>>> +      continue;
>>> +    MatchingPtrPHI = PtrPHI;
>>> +    for (unsigned i = 0; i != PtrPHI->getNumIncomingValues(); ++i) {
>>> +      if (AvailablePtrVals[i] != PtrPHI->getIncomingValue(i)) {
>>> +        MatchingPtrPHI = nullptr;
>>> +        break;
>>> +      }
>>> +    }
>>> +
>>> +    if (MatchingPtrPHI)
>>> +      break;
>>> +  }
>>> +
>>> +  if (MatchingPtrPHI) {
>>> +    assert(MatchingPtrPHI->getType() == IntToPtr->getType() &&
>>> +           "Phi's Type does not match with IntToPtr");
>>> +    // The PtrToCast + IntToPtr will be simplified later
>>> +    return CastInst::CreateBitOrPointerCast(MatchingPtrPHI,
>>> +
>>> IntToPtr->getOperand(0)->getType());
>>> +  }
>>> +
>>> +  // If it requires a conversion for every PHI operand, do not do it.
>>> +  if (std::all_of(AvailablePtrVals.begin(), AvailablePtrVals.end(),
>>> +                  [&](Value *V) {
>>> +                    return (V->getType() != IntToPtr->getType()) ||
>>> +                           isa<IntToPtrInst>(V);
>>> +                  }))
>>> +    return nullptr;
>>> +
>>> +  // If any of the operand that requires casting is a terminator
>>> +  // instruction, do not do it.
>>> +  if (std::any_of(AvailablePtrVals.begin(), AvailablePtrVals.end(),
>>> +                  [&](Value *V) {
>>> +                    return (V->getType() != IntToPtr->getType()) &&
>>> +                           isa<TerminatorInst>(V);
>>> +                  }))
>>> +    return nullptr;
>>> +
>>> +  PHINode *NewPtrPHI = PHINode::Create(
>>> +      IntToPtr->getType(), PN.getNumIncomingValues(), PN.getName() +
>>> ".ptr");
>>> +
>>> +  InsertNewInstBefore(NewPtrPHI, PN);
>>> +  for (unsigned i = 0; i != PN.getNumIncomingValues(); ++i) {
>>> +    auto *IncomingBB = PN.getIncomingBlock(i);
>>> +    auto *IncomingVal = AvailablePtrVals[i];
>>> +
>>> +    if (IncomingVal->getType() == IntToPtr->getType()) {
>>> +      NewPtrPHI->addIncoming(IncomingVal, IncomingBB);
>>> +      continue;
>>> +    }
>>> +
>>> +#ifndef NDEBUG
>>> +    LoadInst *LoadI = dyn_cast<LoadInst>(IncomingVal);
>>> +    assert((isa<PHINode>(IncomingVal) ||
>>> +            IncomingVal->getType()->isPointerTy() ||
>>> +            (LoadI && LoadI->hasOneUse())) &&
>>> +           "Can not replace LoadInst with multiple uses");
>>> +#endif
>>> +    // Need to insert a BitCast.
>>> +    // For an integer Load instruction with a single use, the load +
>>> IntToPtr
>>> +    // cast will be simplified into a pointer load:
>>> +    // %v = load i64, i64* %a.ip, align 8
>>> +    // %v.cast = inttoptr i64 %v to float **
>>> +    // ==>
>>> +    // %v.ptrp = bitcast i64 * %a.ip to float **
>>> +    // %v.cast = load float *, float ** %v.ptrp, align 8
>>> +    auto *CI = CastInst::CreateBitOrPointerCast(
>>> +        IncomingVal, IntToPtr->getType(), IncomingVal->getName() +
>>> ".ptr");
>>> +    if (auto *IncomingI = dyn_cast<Instruction>(IncomingVal)) {
>>> +      BasicBlock::iterator InsertPos(IncomingI);
>>> +      InsertPos++;
>>> +      if (isa<PHINode>(IncomingI))
>>> +        InsertPos = IncomingI->getParent()->getFirstInsertionPt();
>>> +      InsertNewInstBefore(CI, *InsertPos);
>>> +    } else {
>>> +      auto *InsertBB = &IncomingBB->getParent()->getEntryBlock();
>>> +      InsertNewInstBefore(CI, *InsertBB->getFirstInsertionPt());
>>> +    }
>>> +    NewPtrPHI->addIncoming(CI, IncomingBB);
>>> +  }
>>> +
>>> +  // The PtrToCast + IntToPtr will be simplified later
>>> +  return CastInst::CreateBitOrPointerCast(NewPtrPHI,
>>> +                                          IntToPtr->getOperand(0)->getTy
>>> pe());
>>> +}
>>> +
>>>  /// If we have something like phi [add (a,b), add(a,c)] and if a/b/c
>>> and the
>>>  /// adds all have a single use, turn this into a phi and a single binop.
>>>  Instruction *InstCombiner::FoldPHIArgBinOpIntoPHI(PHINode &PN) {
>>> @@ -903,6 +1130,9 @@ Instruction *InstCombiner::visitPHINode(
>>>    // this PHI only has a single use (a PHI), and if that PHI only has
>>> one use (a
>>>    // PHI)... break the cycle.
>>>    if (PN.hasOneUse()) {
>>> +    if (Instruction *Result = FoldIntegerTypedPHI(PN))
>>> +      return Result;
>>> +
>>>      Instruction *PHIUser = cast<Instruction>(PN.user_back());
>>>      if (PHINode *PU = dyn_cast<PHINode>(PHIUser)) {
>>>        SmallPtrSet<PHINode*, 16> PotentiallyDeadPHIs;
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171004/88f09d2c/attachment.html>


More information about the llvm-commits mailing list