[PATCH] D12710: InstCombine: Preserve tbaa metadata when merging loads that are phi arguments

[Akira Hatanaka via llvm-commits]

ahatanak updated this revision to Diff 35029.
ahatanak added a comment.

I was counting on the fact that the incoming loads will eventually get eliminated as they all have one use, so mutating the metadata is safe. But it's probably better to take an approach that doesn't mutate the metadata of an existing load. In the new patch, the metadata is attached to the newly created load.


http://reviews.llvm.org/D12710

Files:
  lib/Transforms/InstCombine/InstCombinePHI.cpp
  test/Transforms/InstCombine/fold-phi-load-tbaa.ll

Index: test/Transforms/InstCombine/fold-phi-load-tbaa.ll
===================================================================
--- /dev/null
+++ test/Transforms/InstCombine/fold-phi-load-tbaa.ll
@@ -0,0 +1,42 @@
+; RUN: opt -instcombine -S < %s | FileCheck %s
+
+%struct.S1 = type { i32, float }
+%struct.S2 = type { float, i32 }
+
+; Check that tbaa metadata is preserved after merging the two loads.
+;
+; CHECK: return:
+; CHECK: load i32, i32* %{{[a-z0-9.]+}}, align 4, !tbaa !0
+
+; Function Attrs: nounwind ssp uwtable
+define i32 @phi_load_tbaa(%struct.S1* %s1, %struct.S2* %s2, i32 %c) #0 {
+entry:
+  %tobool = icmp eq i32 %c, 0
+  br i1 %tobool, label %if.end, label %if.then
+
+if.then:                                          ; preds = %entry
+  %i = getelementptr inbounds %struct.S2, %struct.S2* %s2, i64 0, i32 1
+  %val = load i32, i32* %i, align 4, !tbaa !0
+  br label %return
+
+if.end:                                           ; preds = %entry
+  %i2 = getelementptr inbounds %struct.S1, %struct.S1* %s1, i64 0, i32 0
+  %val2 = load i32, i32* %i2, align 4, !tbaa !2
+  br label %return
+
+return:                                           ; preds = %if.end, %if.then
+  %retval = phi i32 [ %val, %if.then ], [ %val2, %if.end ]
+  ret i32 %retval
+}
+
+; CHECK: !0 = !{!1, !1, i64 0}
+; CHECK: !1 = !{!"int", !{{[0-9]}}, i64 0}
+
+!0 = !{!1, !4, i64 4}
+!1 = !{!"", !7, i64 0, !4, i64 4}
+!2 = !{!3, !4, i64 0}
+!3 = !{!"", !4, i64 0, !7, i64 4}
+!4 = !{!"int", !5, i64 0}
+!5 = !{!"omnipotent char", !6, i64 0}
+!6 = !{!"Simple C/C++ TBAA"}
+!7 = !{!"float", !5, i64 0}
Index: lib/Transforms/InstCombine/InstCombinePHI.cpp
===================================================================
--- lib/Transforms/InstCombine/InstCombinePHI.cpp
+++ lib/Transforms/InstCombine/InstCombinePHI.cpp
@@ -15,6 +15,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/Analysis/InstructionSimplify.h"
+#include "llvm/Transforms/Utils/Local.h"
 using namespace llvm;
 
 #define DEBUG_TYPE "instcombine"
@@ -349,24 +350,27 @@
 
   Value *InVal = FirstLI->getOperand(0);
   NewPN->addIncoming(InVal, PN.getIncomingBlock(0));
+  LoadInst *NewLI = new LoadInst(NewPN, "", isVolatile, LoadAlignment);
+  NewLI->setMetadata(LLVMContext::MD_tbaa,
+                     FirstLI->getMetadata(LLVMContext::MD_tbaa));
 
-  // Add all operands to the new PHI.
+  // Add all operands to the new PHI and combine TBAA metadata.
   for (unsigned i = 1, e = PN.getNumIncomingValues(); i != e; ++i) {
-    Value *NewInVal = cast<LoadInst>(PN.getIncomingValue(i))->getOperand(0);
+    LoadInst *LI = cast<LoadInst>(PN.getIncomingValue(i));
+    combineMetadata(NewLI, LI, { LLVMContext::MD_tbaa });
+    Value *NewInVal = LI->getOperand(0);
     if (NewInVal != InVal)
       InVal = nullptr;
     NewPN->addIncoming(NewInVal, PN.getIncomingBlock(i));
   }
 
-  Value *PhiVal;
   if (InVal) {
     // The new PHI unions all of the same values together.  This is really
     // common, so we handle it intelligently here for compile-time speed.
-    PhiVal = InVal;
+    NewLI->setOperand(0, InVal);
     delete NewPN;
   } else {
     InsertNewInstBefore(NewPN, PN);
-    PhiVal = NewPN;
   }
 
   // If this was a volatile load that we are merging, make sure to loop through
@@ -376,7 +380,6 @@
     for (Value *IncValue : PN.incoming_values())
       cast<LoadInst>(IncValue)->setVolatile(false);
 
-  LoadInst *NewLI = new LoadInst(PhiVal, "", isVolatile, LoadAlignment);
   NewLI->setDebugLoc(FirstLI->getDebugLoc());
   return NewLI;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D12710.35029.patch
Type: text/x-patch
Size: 3567 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150917/79bc48f7/attachment.bin>