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

945 ↗

(On Diff #44801)

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 ↗

(On Diff #44929)

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

[• hulx2000]

lib/CodeGen/IslNodeBuilder.cpp

923 ↗

(On Diff #44929)

Oh, sorry, I missed that one too.

[• hulx2000]

updated again

polly/trunk/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;

polly/trunk/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());
+  if (LoadInst *PreloadInst = dyn_cast<LoadInst>(PreloadVal))
+    PreloadInst->setAlignment(dyn_cast<LoadInst>(AccInst)->getAlignment());
+
   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);
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);

polly/trunk/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
+}
+