[PATCH] D24980: [GlobalOpt]: See if it's possible to shrink Stores with Undef during global variable initialization to eliminate constructor functions

[David Majnemer via llvm-commits]

majnemer added a comment.

Please use clang-format on your diff.



> Evaluator.cpp:253
>                  Ptr = FoldedPtr;
> -
> -            // If we can't improve the situation by introspecting NewTy,
> -            // we have to give up.
> +            } else if (ArrayType *ATy = dyn_cast<ArrayType>(NewTy)) {
> +              // See if we can remove undefs at the end of a vector to make

`auto *`

> Evaluator.cpp:263-264
> +              // <4 x float> *)
> +              VectorType *ElemTy = dyn_cast<VectorType>(ATy->getElementType());
> +              ConstantVector *Vec = dyn_cast<ConstantVector>(Val);
> +              if (Vec && ElemTy) {

`auto *`

> Evaluator.cpp:269
> +                unsigned ValSize = Vec->getNumOperands();
> +                // Make sure we're truncating and not extending
> +                if ( ValSize <= ElemTy->getNumElements())

Please end comments with a period.

> Evaluator.cpp:272-277
> +                for (unsigned i = ValSize; i > ElemTy->getNumElements(); --i) {
> +                  Value *V = Vec->getOperand(i - 1);
> +                  if (!dyn_cast<UndefValue>(V))
> +                    // This is not an undef and we can't shrink the store
> +                    return false;
> +                }

This loop looks a little confusing, why not loop from `ElemTy->getNumElements()` to `ValSize`?

> Evaluator.cpp:274
> +                  Value *V = Vec->getOperand(i - 1);
> +                  if (!dyn_cast<UndefValue>(V))
> +                    // This is not an undef and we can't shrink the store

`!isa<UndefValue>(V)`

> ShrinkUndefStore.ll:1
> +; RUN: opt -o - %s -globalopt -S | FileCheck %s
> +; CHECK-NOT: ctor1

It is more common to see:

  ; RUN: opt -globalopt -S < %s | FileCheck %s

> ShrinkUndefStore.ll:2
> +; RUN: opt -o - %s -globalopt -S | FileCheck %s
> +; CHECK-NOT: ctor1
> + at _ZL9aPosition = internal addrspace(2) global [1 x <3 x float>] zeroinitializer, align 16

Please have more thorough CHECKs.

> ShrinkUndefStore.ll:18-26
> +define internal fastcc void @__cxx_global_var_init() #0 section "__TEXT,__StaticInit,regular,pure_instructions" {
> +entry:
> +  %arrayidx = getelementptr inbounds [1 x <3 x float>], [1 x <3 x float>] addrspace(2)* @_ZL9aPosition, i32 0, i64 0
> +  store <4 x float> <float 4.000000e+00, float 4.000000e+00, float 4.000000e+00, float undef>, <4 x float> addrspace(2)* bitcast ([1 x <3 x float>] addrspace(2)* @_ZL9aPosition to <4 x float> addrspace(2)*), !tbaa !7
> +  ret void
> +}
> +

Please remove these Mach-O section attributes.

Repository:
  rL LLVM

https://reviews.llvm.org/D24980