Make sure we preserve alignment information after hoisting invariant load

[• hulx2000]

Summary

In polly, after hoisting loop invariant loads outside loop, the alignment information for hoisted loads are missing, this patch restore them.

[jdoerfert]

Suggestion for a small improvement.

lib/CodeGen/IslNodeBuilder.cpp

950

Why do you try to find LoadInst after the call to "preloadUnconditionally" and set the alignment there instead of in the ditonally function when the load is actually created?

I would suggest to change the last argument from AccInstTy to AccInst and set the alignment then inside the function. What do you say?

[• hulx2000]

I agree, that's better, I will update that.

[• hulx2000]

updated to address Johannes's comment, thanks Johannes

[jdoerfert]

LGTM. You can commit this or tell me if I should commit it. Thanks for the update!

[jdoerfert]

lib/CodeGen/IslNodeBuilder.cpp

923

I forgot. We should set the alingment here too. Sorry.

[• hulx2000]

lib/CodeGen/IslNodeBuilder.cpp

923

Oh, sorry, I missed that one too.

[• hulx2000]

updated again

include/polly/CodeGen/IslNodeBuilder.h

protected:
 
   /// @brief Set to remember materialized invariant loads.
   SmallPtrSet<const SCEV *, 16> PreloadedPtrs;
 
   /// @brief Preload the memory access at @p AccessRange with @p Build.
   ///
   /// @returns The preloaded value casted to type @p Ty
   Value *preloadUnconditionally(__isl_take isl_set *AccessRange,
-                                isl_ast_build *Build, Type *Ty);
+                                isl_ast_build *Build, Instruction *AccInst);
 
   /// @brief Preload the memory load access @p MA.
   ///
   /// If @p MA is not always executed it will be conditionally loaded and
   /// merged with undef from the same type. Hence, if @p MA is executed only
   /// under condition C then the preload code will look like this:
   ///
   /// MA_preload = undef;

lib/CodeGen/IslNodeBuilder.cpp

for (unsigned i = 0, e = isl_set_dim(Set, isl_dim_param); i < e; ++i) {
     isl_id *Id = isl_set_get_dim_id(Set, isl_dim_param, i);
     if (!materializeValue(Id))
       return false;
   }
   return true;
 }
 
 Value *IslNodeBuilder::preloadUnconditionally(isl_set *AccessRange,
-                                              isl_ast_build *Build, Type *Ty) {
+                                              isl_ast_build *Build,
+                                              Instruction *AccInst) {
   isl_pw_multi_aff *PWAccRel = isl_pw_multi_aff_from_set(AccessRange);
   PWAccRel = isl_pw_multi_aff_gist_params(PWAccRel, S.getContext());
   isl_ast_expr *Access =
       isl_ast_build_access_from_pw_multi_aff(Build, PWAccRel);
   Value *PreloadVal = ExprBuilder.create(Access);
 
+  if (LoadInst *PreloadInst = dyn_cast<LoadInst>(PreloadVal))
+    PreloadInst->setAlignment(dyn_cast<LoadInst>(AccInst)->getAlignment());
+
   // Correct the type as the SAI might have a different type than the user
   // expects, especially if the base pointer is a struct.
+  Type *Ty = AccInst->getType();
   if (Ty == PreloadVal->getType())
     return PreloadVal;
 
   if (!Ty->isFloatingPointTy() && !PreloadVal->getType()->isFloatingPointTy())
     return PreloadVal = Builder.CreateBitOrPointerCast(PreloadVal, Ty);
 
   // We do not want to cast floating point to non-floating point types and vice
   // versa, thus we simply create a new load with a casted pointer expression.
   auto *LInst = dyn_cast<LoadInst>(PreloadVal);
   assert(LInst && "Preloaded value was not a load instruction");
   auto *Ptr = LInst->getPointerOperand();
   Ptr = Builder.CreatePointerCast(Ptr, Ty->getPointerTo(),
                                   Ptr->getName() + ".cast");
   PreloadVal = Builder.CreateLoad(Ptr, LInst->getName());
   LInst->eraseFromParent();
   return PreloadVal;
 }
 
 Value *IslNodeBuilder::preloadInvariantLoad(const MemoryAccess &MA,
                                             isl_set *Domain) {
 
   isl_set *AccessRange = isl_map_range(MA.getAccessRelation());
   if (!materializeParameters(AccessRange, false)) {
     isl_set_free(AccessRange);
     isl_set_free(Domain);
     return nullptr;
   }
 
   auto *Build = isl_ast_build_from_context(isl_set_universe(S.getParamSpace()));
   isl_set *Universe = isl_set_universe(isl_set_get_space(Domain));
   bool AlwaysExecuted = isl_set_is_equal(Domain, Universe);
   isl_set_free(Universe);
 
   Instruction *AccInst = MA.getAccessInstruction();
   Type *AccInstTy = AccInst->getType();
 
   Value *PreloadVal = nullptr;
   if (AlwaysExecuted) {
-    PreloadVal = preloadUnconditionally(AccessRange, Build, AccInstTy);
+    PreloadVal = preloadUnconditionally(AccessRange, Build, AccInst);
     isl_ast_build_free(Build);
     isl_set_free(Domain);
     return PreloadVal;
   }
 
   if (!materializeParameters(Domain, false)) {
     isl_ast_build_free(Build);
     isl_set_free(AccessRange);
     isl_set_free(Domain);
     return nullptr;
Value *IslNodeBuilder::preloadInvariantLoad(const MemoryAccess &MA,
   Builder.SetInsertPoint(CondBBTerminator);
   Builder.CreateCondBr(Cond, ExecBB, MergeBB);
   CondBBTerminator->eraseFromParent();
 
   Builder.SetInsertPoint(ExecBB);
   Builder.CreateBr(MergeBB);
 
   Builder.SetInsertPoint(ExecBB->getTerminator());
-  Value *PreAccInst = preloadUnconditionally(AccessRange, Build, AccInstTy);
-
+  Value *PreAccInst = preloadUnconditionally(AccessRange, Build, AccInst);
   Builder.SetInsertPoint(MergeBB->getTerminator());
   auto *MergePHI = Builder.CreatePHI(
       AccInstTy, 2, "polly.preload." + AccInst->getName() + ".merge");
   MergePHI->addIncoming(PreAccInst, ExecBB);
   MergePHI->addIncoming(Constant::getNullValue(AccInstTy), CondBB);
   PreloadVal = MergePHI;
 
   isl_ast_build_free(Build);

test/Isl/CodeGen/invaraint_load_hoist_alignment.ll

+; RUN: opt %loadPolly -basicaa -polly-codegen -polly-vectorizer=polly -S %s | FileCheck %s
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
+target triple = "x86_64-unknown-linux-gnu"
+
+@A = common global [1024 x i32] zeroinitializer, align 16
+@B = common global [1024 x i32] zeroinitializer, align 16
+
+declare i32 @foo(i32) readnone
+
+define void @force_alignment() nounwind {
+;CHECK: @force_alignment
+;CHECK-FORCED: @force_alignment
+entry:
+  br label %body
+
+body:
+  %indvar = phi i64 [ 0, %entry ], [ %indvar_next, %body ]
+  %scevgep = getelementptr [1024 x i32], [1024 x i32]* @B, i64 0, i64 %indvar
+; CHECK: [[T2:%.load]] = load i32, i32* getelementptr inbounds ([1024 x i32], [1024 x i32]* @A, i32 0, i32 0), align 4
+; CHECK: %value_p.splatinsert = insertelement <4 x i32> undef, i32 [[T2]], i32 0
+  %value = load i32, i32* getelementptr inbounds ([1024 x i32], [1024 x i32]* @A, i64 0, i64 0), align 4
+  %result = tail call i32 @foo(i32 %value) nounwind
+  store i32 %result, i32* %scevgep, align 4
+  %indvar_next = add i64 %indvar, 1
+  %exitcond = icmp eq i64 %indvar_next, 4
+  br i1 %exitcond, label %return, label %body
+
+return:
+  ret void
+}
+