Upgrade TBAA *before* upgrading intrinsics

[beanz]

Summary

If TBAA is on an intrinsic and it gets upgraded and drops the TBAA we hit an odd assert. We should just upgrade the TBAA first because it doesn't have side-effects.

[reames]

Confusing comment needs addressed, otherwise, looks straight forward. LGTM w/comment addressed.

lib/Bitcode/Reader/BitcodeReader.cpp

5337 ↗

(On Diff #50877)

your comment makes this sound optional. Given this is fixing a bug, is that true?

llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp

if (LastFunctionBlockBit || NextUnreadBit)
     parseModule(LastFunctionBlockBit > NextUnreadBit ? LastFunctionBlockBit
                                                      : NextUnreadBit);
 
   // Check that all block address forward references got resolved (as we
   // promised above).
   if (!BasicBlockFwdRefs.empty())
     return error("Never resolved function from blockaddress");
 
+  // Upgrading intrinsic calls before TBAA can cause TBAA metadata to be lost,
+  // to prevent this instructions with TBAA tags should be upgraded first.
+  for (unsigned I = 0, E = InstsWithTBAATag.size(); I < E; I++)
+    UpgradeInstWithTBAATag(InstsWithTBAATag[I]);
+
   // Upgrade any intrinsic calls that slipped through (should not happen!) and
   // delete the old functions to clean up. We can't do this unless the entire
   // module is materialized because there could always be another function body
   // with calls to the old function.
   for (auto &I : UpgradedIntrinsics) {
     for (auto *U : I.first->users()) {
       if (CallInst *CI = dyn_cast<CallInst>(U))
         UpgradeIntrinsicCall(CI, I.second);
     }
     if (!I.first->use_empty())
       I.first->replaceAllUsesWith(I.second);
     I.first->eraseFromParent();
   }
   UpgradedIntrinsics.clear();
 
-  for (unsigned I = 0, E = InstsWithTBAATag.size(); I < E; I++)
-    UpgradeInstWithTBAATag(InstsWithTBAATag[I]);
-
   UpgradeDebugInfo(*TheModule);
   return std::error_code();
 }
 
 std::vector<StructType *> BitcodeReader::getIdentifiedStructTypes() const {
   return IdentifiedStructTypes;
 }
 

llvm/trunk/test/Assembler/auto_upgrade_intrinsics.ll

; CHECK: @llvm.objectsize.i32.p0i8
   ret i32 %s
 }
 
 declare <2 x double> @llvm.masked.load.v2f64(<2 x double>* %ptrs, i32, <2 x i1> %mask, <2 x double> %src0)
 
 define <2 x double> @tests.masked.load(<2 x double>* %ptr, <2 x i1> %mask, <2 x double> %passthru)  {
 ; CHECK-LABEL: @tests.masked.load(
 ; CHECK: @llvm.masked.load.v2f64.p0v2f64
-  %res = call <2 x double> @llvm.masked.load.v2f64(<2 x double>* %ptr, i32 1, <2 x i1> %mask, <2 x double> %passthru)
+  %res = call <2 x double> @llvm.masked.load.v2f64(<2 x double>* %ptr, i32 1, <2 x i1> %mask, <2 x double> %passthru), !tbaa !0
   ret <2 x double> %res
 }
 
 declare void @llvm.masked.store.v2f64(<2 x double> %val, <2 x double>* %ptrs, i32, <2 x i1> %mask)
 
 define void @tests.masked.store(<2 x double>* %ptr, <2 x i1> %mask, <2 x double> %val)  {
 ; CHECK-LABEL: @tests.masked.store(
 ; CHECK: @llvm.masked.store.v2f64.p0v2f64
-  call void @llvm.masked.store.v2f64(<2 x double> %val, <2 x double>* %ptr, i32 3, <2 x i1> %mask)
+  call void @llvm.masked.store.v2f64(<2 x double> %val, <2 x double>* %ptr, i32 3, <2 x i1> %mask), !tbaa !0
   ret void
-}
+}
+
+!0 = !{!"omnipotent char", !1}
+!1 = !{!"Simple C/C++ TBAA"}
+!2 = !{!"double", !0}