[llvm] [SimplifyCFG] Enable sinking instructions with differing alignments (PR #215858)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-llvm-transforms

Author: Sayan Sivakumaran (sivakusayan)

<details>
<summary>Changes</summary>

Fixes https://github.com/llvm/llvm-project/issues/45117.

Previously, we would not sink stores if the candidates for sinking had different alignments. However, we can still easily sink these stores if we can find a common alignment for all store instructions. It suffices to take the smallest alignment as alignments must be powers of 2. The hope is that additional opportunities for simplifying the CFG will make up for lost alignment information in the majority of cases.

Note that this patch will automatically fix the same problem for loads.

---
Full diff: https://github.com/llvm/llvm-project/pull/215858.diff


4 Files Affected:

- (modified) llvm/include/llvm/IR/Instruction.h (+7) 
- (modified) llvm/lib/IR/Instruction.cpp (+18) 
- (modified) llvm/lib/Transforms/Utils/SimplifyCFG.cpp (+3-2) 
- (modified) llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll (+50) 


``````````diff
diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index 13646ec66d6b3..c6c9e0eb0e637 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -724,6 +724,13 @@ class Instruction : public User,
   /// V and this instruction.
   LLVM_ABI void andIRFlags(const Value *V);
 
+  /// Restricts this instruction's attributes to those which are shared
+  /// with I. This will essentially:
+  ///  1) Do a logical 'and' of this instruction's flags with I's flags.
+  ///  2) Find a common alignment with I, if both instructions support a notion
+  ///  of alignment.
+  LLVM_ABI void intersectIRAttributes(const Instruction *I);
+
   /// Merge 2 debug locations and apply it to the Instruction. If the
   /// instruction is a CallIns, we need to traverse the inline chain to find
   /// the common scope. This is not efficient for N-way merging as each time
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index 099a54e684c96..84c6ac0d20dfb 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -813,6 +813,24 @@ void Instruction::andIRFlags(const Value *V) {
       DestICmp->setSameSign(DestICmp->hasSameSign() && SrcICmp->hasSameSign());
 }
 
+void Instruction::intersectIRAttributes(const Instruction *I) {
+  // TODO: Should we intersect metadata here too?
+  andIRFlags(I);
+
+  auto IsLoadOrStore = [](const Instruction *I) {
+    return isa<LoadInst>(I) || isa<StoreInst>(I);
+  };
+  if (IsLoadOrStore(I) && IsLoadOrStore(this)) {
+    Align Common =
+        std::min(getLoadStoreAlignment(this), getLoadStoreAlignment(I));
+    assert(isAligned(Common, getLoadStoreAlignment(this).value()) &&
+           isAligned(Common, getLoadStoreAlignment(I).value()) &&
+           "Common alignment doesn't satisfy alignment constraints?");
+
+    setLoadStoreAlignment(this, Common);
+  }
+}
+
 const char *Instruction::getOpcodeName(unsigned OpCode) {
   switch (OpCode) {
   // Terminators
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 089892dc573f3..e29a682154e69 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -2248,7 +2248,8 @@ static bool canSinkInstructions(
   const Instruction *I0 = Insts.front();
   const auto I0MMRA = MMRAMetadata(*I0);
   for (auto *I : Insts) {
-    if (!I->isSameOperationAs(I0, Instruction::CompareUsingIntersectedAttrs))
+    if (!I->isSameOperationAs(I0, Instruction::CompareUsingIntersectedAttrs |
+                                      Instruction::CompareIgnoringAlignment))
       return false;
 
     // Treat MMRAs conservatively. This pass can be quite aggressive and
@@ -2379,7 +2380,7 @@ static void sinkLastInstruction(ArrayRef<BasicBlock*> Blocks) {
       // instead of using complex API for N-way merge.
       I0->applyMergedLocation(I0->getDebugLoc(), I->getDebugLoc());
       combineMetadataForCSE(I0, I, true);
-      I0->andIRFlags(I);
+      I0->intersectIRAttributes(I);
       if (auto *CB = dyn_cast<CallBase>(I0)) {
         bool Success = CB->tryIntersectAttributes(cast<CallBase>(I));
         assert(Success && "We should not be trying to sink callbases "
diff --git a/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll b/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
index 8daa2334a4a68..e9d362f2d8538 100644
--- a/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
+++ b/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
@@ -2177,6 +2177,56 @@ join:
   ret i32 %phi
 }
 
+define void @store_different_alignments(i1 %c, ptr %p1, ptr %p2) {
+; CHECK-LABEL: @store_different_alignments(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[P2:%.*]] = select i1 [[C:%.*]], ptr [[P1:%.*]], ptr [[P3:%.*]]
+; CHECK-NEXT:    store i8 0, ptr [[P2]], align 1
+; CHECK-NEXT:    ret void
+;
+entry:
+  br i1 %c, label %if1, label %if2
+
+if1:
+  store i8 0, ptr %p1, align 2
+  br label %end
+
+if2:
+  store i8 0, ptr %p2, align 1
+  br label %end
+
+end:
+  ret void
+}
+
+define void @store_different_alignments_incompatible_attributes(i1 %c, ptr %p1, ptr %p2) {
+; CHECK-LABEL: @store_different_alignments_incompatible_attributes(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br i1 [[C:%.*]], label [[IF1:%.*]], label [[IF2:%.*]]
+; CHECK:       if1:
+; CHECK-NEXT:    store volatile i8 0, ptr [[P1:%.*]], align 2
+; CHECK-NEXT:    br label [[END:%.*]]
+; CHECK:       if2:
+; CHECK-NEXT:    store i8 0, ptr [[P2:%.*]], align 1
+; CHECK-NEXT:    br label [[END]]
+; CHECK:       end:
+; CHECK-NEXT:    ret void
+;
+entry:
+  br i1 %c, label %if1, label %if2
+
+if1:
+  store volatile i8 0, ptr %p1, align 2
+  br label %end
+
+if2:
+  store i8 0, ptr %p2, align 1
+  br label %end
+
+end:
+  ret void
+}
+
 declare void @dummy()
 declare void @use.ptr(ptr)
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/215858