[llvm] [SimplifyCFG] Drop UB-implying metadata when hoisting past side effects (PR #226746)

[Nikita Popov via llvm-commits]

================
@@ -2053,6 +2053,20 @@ bool SimplifyCFGOpt::hoistCommonCodeFromSuccessors(Instruction *TI,
         I1->applyMergedLocation(I1->getDebugLoc(), I2->getDebugLoc());
         I2->eraseFromParent();
       }
+      // I1 now executes before the instructions we skipped.
+      unsigned SkippedFlags = 0;
+      for (const SuccIterPair &P : SuccIterPairs)
+        SkippedFlags |= P.second;
+      if (SkippedFlags & SkipImplicitControlFlow) {
+        // One of them may throw or not return, so I1 is speculated.
+        I1->dropUBImplyingAttrsAndMetadata();
+      } else if (SkippedFlags & SkipSideEffect) {
+        // One of them may write memory, for example allocate or free it, so
+        // metadata that only holds at I1's old position may not hold here.
+        I1->setMetadata(LLVMContext::MD_dereferenceable, nullptr);
+        I1->setMetadata(LLVMContext::MD_dereferenceable_or_null, nullptr);
----------------
nikic wrote:

I believe we should leave the dereferenceable cases alone. If everything else is correct, it is impossible to hoist a `!dereferenceable` instruction past the instruction that allocates the dereferenceable memory. In the normal case, there must be some form of use-def relationship between the allocation and the instruction with !dereferenceable that would prevent hoisting. In the case of exposed provenance, there must be a ptrtoint expose effect past which the inttoptr cannot be hoisted.

We currently fail to model the exposure memory effects, but that's a larger problem. Once that is fixed, this problem is fixed as well. I think that trying to work around it here just confuses the situation.

(Some for nofreeobj.)

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