Preserving 'nonnull' metadata in SimplifyCFG

[reames]

Summary

The actual change in this review are trivial (1 line), but I'd like a second opinion on their legality from someone familiar with the context. I *believe* that this is legal, but a confirmation would help. The semantics we're going for are: 'nonnull' is preserved when two instructions are merged iff both have it.

Also, anyone know a reason we're not merging metadata when sinking instructions but are merging when hoisting?

[hfinkel]

Full context patch please ;)

Generally speaking, I think it must be legal to preserve nonnull anywhere that preserving range metadata is preserved. Since we're hoisting common code to both sides of the branch, there can't be any control-dependency issues here, and I think that the preservation is correct.

LGTM.

lib/Transforms/Utils/SimplifyCFG.cpp

1315 ↗

(On Diff #15246)

Yes, it looks like we should be doing this. Since this is common-code combining, I think there are no control dependency issues here (feel free to do so).

[reames]

Closed by commit rL220392 (authored by @reames).

llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp

do {
     BIParent->getInstList().splice(BI, BB1->getInstList(), I1);
     if (!I2->use_empty())
       I2->replaceAllUsesWith(I1);
     I1->intersectOptionalDataWith(I2);
     unsigned KnownIDs[] = {
       LLVMContext::MD_tbaa,
       LLVMContext::MD_range,
       LLVMContext::MD_fpmath,
-      LLVMContext::MD_invariant_load
+      LLVMContext::MD_invariant_load,
+      LLVMContext::MD_nonnull
     };
     combineMetadata(I1, I2, KnownIDs);
     I2->eraseFromParent();
     Changed = true;
 
     I1 = BB1_Itr++;
     I2 = BB2_Itr++;
     // Skip debug info if it is not identical.
while (RI1 != RE1 && RI2 != RE2) {
     BBEnd->getInstList().splice(FirstNonPhiInBBEnd, BB1->getInstList(), I1);
     if (!OldPN->use_empty())
       OldPN->replaceAllUsesWith(I1);
     OldPN->eraseFromParent();
 
     if (!I2->use_empty())
       I2->replaceAllUsesWith(I1);
     I1->intersectOptionalDataWith(I2);
+    // TODO: Use combineMetadata here to preserve what metadata we can
+    // (analogous to the hoisting case above).
     I2->eraseFromParent();
 
     if (UpdateRE1)
       RE1 = BB1->getInstList().rend();
     if (UpdateRE2)
       RE2 = BB2->getInstList().rend();
     FirstNonPhiInBBEnd = I1;
     NumSinkCommons++;