[DeadArgElim] Do not optimize builtin annotated calls.

[jeroen.dobbelaere]

Summary

Do not optimize builtin annotated calls, based on the visibility on a
possible implementation. Other optimization passes might still need the
operand.

This fixes https://bugs.llvm.org/show_bug.cgi?id=49022

[jeroen.dobbelaere]

This is an attempt to trigger the discussion. This patch fixes the observed issue, but it might not be the right place to do ?

[fhahn]

In D97672#2593819, @jeroen.dobbelaere wrote:

This is an attempt to trigger the discussion. This patch fixes the observed issue, but it might not be the right place to do ?

I am not sure if that's the right place to fix it. It seems like we cannot really trust builtin in this case and there probably are other places that would also need fixing?

[bjope]

For info: Looks like https://github.com/llvm/llvm-project/issues/48366 (aka https://bugs.llvm.org/show_bug.cgi?id=49022) is closed nowadays. Solved by D97735.

llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp

bool DeadArgumentEliminationPass::RemoveDeadArgumentsFromCallers(Function &Fn) {
   if (UnusedArgs.empty())
     return false;
 
   for (Use &U : Fn.uses()) {
     CallBase *CB = dyn_cast<CallBase>(U.getUser());
     if (!CB || !CB->isCallee(&U))
       continue;
 
+    // Don't touch builtin annotated calls. The analysis might have seen a
+    // (nobuiltin) implementation that allows to optimize an argument, but other
+    // optimizations of builtin calls might still depend on that argument.
+    if (CB->hasFnAttr(Attribute::Builtin))
+      continue;
+
     // Now go through all unused args and replace them with "undef".
     for (unsigned I = 0, E = UnusedArgs.size(); I != E; ++I) {
       unsigned ArgNo = UnusedArgs[I];
 
       Value *Arg = CB->getArgOperand(ArgNo);
       CB->setArgOperand(ArgNo, UndefValue::get(Arg->getType()));
       ++NumArgumentsReplacedWithUndef;
       Changed = true;

llvm/test/Transforms/DeadArgElim/builtin_functions.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature
+; RUN: opt -S -deadargelim %s | FileCheck %s
+
+; Function Attrs: nobuiltin nounwind uwtable mustprogress
+define dso_local void @_ZdlPv(i8* %0) #0 {
+; CHECK-LABEL: define {{[^@]+}}@_ZdlPv
+; CHECK-SAME: (i8* [[TMP0:%.*]]) [[ATTR0:#.*]] {
+; CHECK-NEXT:    ret void
+;
+  ret void
+}
+
+; The arguments for builtin annotated call must not be replaced
+define void @use_builtin_delete(i8* %p) {
+; CHECK-LABEL: define {{[^@]+}}@use_builtin_delete
+; CHECK-SAME: (i8* [[P:%.*]]) {
+; CHECK-NEXT:    call void @_ZdlPv(i8* [[P]]) [[ATTR1:#.*]]
+; CHECK-NEXT:    ret void
+;
+  call void @_ZdlPv(i8* %p) #1
+  ret void
+}
+
+define void @use_other_delete(i8* %p) {
+; CHECK-LABEL: define {{[^@]+}}@use_other_delete
+; CHECK-SAME: (i8* [[P:%.*]]) {
+; CHECK-NEXT:    call void @_ZdlPv(i8* undef) [[ATTR2:#.*]]
+; CHECK-NEXT:    ret void
+;
+  call void @_ZdlPv(i8* %p) #2
+  ret void
+}
+
+attributes #0 = { nobuiltin nounwind uwtable mustprogress readnone willreturn }
+attributes #1 = { builtin nounwind }
+attributes #2 = { nounwind }