[llvm] r199771 - [SROA] Fix a bug which could cause the common type finding to return

[Chandler Carruth]

Sorry, I forgot to mention in the commit log -- there's no test case as
triggering this requires very fragile ordering of operations and so wasn't
stable. (This is from James's original patch.)


On Tue, Jan 21, 2014 at 3:16 PM, Chandler Carruth <chandlerc at gmail.com>wrote:

> Author: chandlerc
> Date: Tue Jan 21 17:16:05 2014
> New Revision: 199771
>
> URL: http://llvm.org/viewvc/llvm-project?rev=199771&view=rev
> Log:
> [SROA] Fix a bug which could cause the common type finding to return
> inconsistent results for different orderings of alloca slices. The
> fundamental issue is that it is just always a mistake to return early
> from this function. There is no effective early exit to leverage. This
> patch stops trynig to do so and simplifies the code a bit as
> a consequence.
>
> Original diagnosis and patch by James Molloy with some name tweaks by me
> in part reflecting feedback from Duncan Smith on the mailing list.
>
> Modified:
>     llvm/trunk/lib/Transforms/Scalar/SROA.cpp
>
> Modified: llvm/trunk/lib/Transforms/Scalar/SROA.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SROA.cpp?rev=199771&r1=199770&r2=199771&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/SROA.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/SROA.cpp Tue Jan 21 17:16:05 2014
> @@ -957,7 +957,11 @@ static Type *findCommonType(AllocaSlices
>                              AllocaSlices::const_iterator E,
>                              uint64_t EndOffset) {
>    Type *Ty = 0;
> -  bool IgnoreNonIntegralTypes = false;
> +  bool TyIsCommon = true;
> +  IntegerType *ITy = 0;
> +
> +  // Note that we need to look at *every* alloca slice's Use to ensure we
> +  // always get consistent results regardless of the order of slices.
>    for (AllocaSlices::const_iterator I = B; I != E; ++I) {
>      Use *U = I->getUse();
>      if (isa<IntrinsicInst>(*U->getUser()))
> @@ -970,37 +974,30 @@ static Type *findCommonType(AllocaSlices
>        UserTy = LI->getType();
>      } else if (StoreInst *SI = dyn_cast<StoreInst>(U->getUser())) {
>        UserTy = SI->getValueOperand()->getType();
> -    } else {
> -      IgnoreNonIntegralTypes = true; // Give up on anything but an iN
> type.
> -      continue;
>      }
>
> -    if (IntegerType *ITy = dyn_cast<IntegerType>(UserTy)) {
> +    if (!UserTy || (Ty && Ty != UserTy))
> +      TyIsCommon = false; // Give up on anything but an iN type.
> +    else
> +      Ty = UserTy;
> +
> +    if (IntegerType *UserITy = dyn_cast_or_null<IntegerType>(UserTy)) {
>        // If the type is larger than the partition, skip it. We only
> encounter
>        // this for split integer operations where we want to use the type
> of the
>        // entity causing the split. Also skip if the type is not a byte
> width
>        // multiple.
> -      if (ITy->getBitWidth() % 8 != 0 ||
> -          ITy->getBitWidth() / 8 > (EndOffset - B->beginOffset()))
> +      if (UserITy->getBitWidth() % 8 != 0 ||
> +          UserITy->getBitWidth() / 8 > (EndOffset - B->beginOffset()))
>          continue;
>
> -      // If we have found an integer type use covering the alloca, use
> that
> -      // regardless of the other types, as integers are often used for
> -      // a "bucket of bits" type.
> -      //
> -      // NB: This *must* be the only return from inside the loop so that
> the
> -      // order of slices doesn't impact the computed type.
> -      return ITy;
> -    } else if (IgnoreNonIntegralTypes) {
> -      continue;
> +      // Track the largest bitwidth integer type used in this way in case
> there
> +      // is no common type.
> +      if (!ITy || ITy->getBitWidth() < UserITy->getBitWidth())
> +        ITy = UserITy;
>      }
> -
> -    if (Ty && Ty != UserTy)
> -      IgnoreNonIntegralTypes = true; // Give up on anything but an iN
> type.
> -
> -    Ty = UserTy;
>    }
> -  return Ty;
> +
> +  return TyIsCommon ? Ty : ITy;
>  }
>
>  /// PHI instructions that use an alloca and are subsequently loaded can be
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140121/60bec4d4/attachment.html>