[ObjCARC] Drop nullary clang.arc.attachedcall bundles in autoupgrade.

[ab]

Summary

This was originally part of D118214, and was removed because we didn't know of situations where the post-arc-contract nullary bundle ends up in bitcode. Turns out this does happen, with swiftc -embed-bitcode!

In certain use-cases, these can be emitted by old compilers, but the
operand is now always required.  These are only used for optimizations,
so it's safe to drop them if they happen to have the now-invalid format.
The semantically-required call is already a separate instruction.

[ahatanak]

LGTM

llvm/include/llvm/IR/AutoUpgrade.h

 //===- AutoUpgrade.h - AutoUpgrade Helpers ----------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  These functions are implemented by lib/IR/AutoUpgrade.cpp.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_IR_AUTOUPGRADE_H
 #define LLVM_IR_AUTOUPGRADE_H
 
 #include "llvm/ADT/StringRef.h"
+#include <vector>
 
 namespace llvm {
   class AttrBuilder;
   class CallBase;
   class Constant;
   class Function;
   class Instruction;
   class GlobalVariable;
   class MDNode;
   class Module;
   class StringRef;
   class Type;
   class Value;
 
+  template <typename T> class OperandBundleDefT;
+  using OperandBundleDef = OperandBundleDefT<Value *>;
+
   /// This is a more granular function that simply checks an intrinsic function
   /// for upgrading, and returns true if it requires upgrading. It may return
   /// null in NewFn if the all calls to the original intrinsic function
   /// should be transformed to non-function-call instructions.
   bool UpgradeIntrinsicFunction(Function *F, Function *&NewFn);
 
   /// This is the complement to the above, replacing a specific call to an
   /// intrinsic function with a call to the specified new function.
namespace llvm {
 
   /// Upgrade the datalayout string by adding a section for address space
   /// pointers.
   std::string UpgradeDataLayoutString(StringRef DL, StringRef Triple);
 
   /// Upgrade attributes that changed format or kind.
   void UpgradeAttributes(AttrBuilder &B);
 
+  /// Upgrade operand bundles (without knowing about their user instruction).
+  void UpgradeOperandBundles(std::vector<OperandBundleDef> &OperandBundles);
+
 } // End llvm namespace
 
 #endif

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

case bitc::FUNC_CODE_INST_INVOKE: {
           unsigned OpTypeID;
           if (getValueTypePair(Record, OpNum, NextValueNo, Op, OpTypeID))
             return error("Invalid record");
           Ops.push_back(Op);
           ArgTyIDs.push_back(OpTypeID);
         }
       }
 
+      // Upgrade the bundles if needed.
+      if (!OperandBundles.empty())
+        UpgradeOperandBundles(OperandBundles);
+
       I = InvokeInst::Create(FTy, Callee, NormalBB, UnwindBB, Ops,
                              OperandBundles);
       ResTypeID = getContainedTypeID(FTyID);
       OperandBundles.clear();
       InstructionList.push_back(I);
       cast<InvokeInst>(I)->setCallingConv(
           static_cast<CallingConv::ID>(CallingConv::MaxID & CCInfo));
       cast<InvokeInst>(I)->setAttributes(PAL);
case bitc::FUNC_CODE_INST_CALLBR: {
           unsigned OpTypeID;
           if (getValueTypePair(Record, OpNum, NextValueNo, Op, OpTypeID))
             return error("Invalid record");
           Args.push_back(Op);
           ArgTyIDs.push_back(OpTypeID);
         }
       }
 
+      // Upgrade the bundles if needed.
+      if (!OperandBundles.empty())
+        UpgradeOperandBundles(OperandBundles);
+
       I = CallBrInst::Create(FTy, Callee, DefaultDest, IndirectDests, Args,
                              OperandBundles);
       ResTypeID = getContainedTypeID(FTyID);
       OperandBundles.clear();
       InstructionList.push_back(I);
       cast<CallBrInst>(I)->setCallingConv(
           static_cast<CallingConv::ID>((0x7ff & CCInfo) >> bitc::CALL_CCONV));
       cast<CallBrInst>(I)->setAttributes(PAL);
case bitc::FUNC_CODE_INST_CALL: {
           unsigned OpTypeID;
           if (getValueTypePair(Record, OpNum, NextValueNo, Op, OpTypeID))
             return error("Invalid record");
           Args.push_back(Op);
           ArgTyIDs.push_back(OpTypeID);
         }
       }
 
+      // Upgrade the bundles if needed.
+      if (!OperandBundles.empty())
+        UpgradeOperandBundles(OperandBundles);
+
       I = CallInst::Create(FTy, Callee, Args, OperandBundles);
       ResTypeID = getContainedTypeID(FTyID);
       OperandBundles.clear();
       InstructionList.push_back(I);
       cast<CallInst>(I)->setCallingConv(
           static_cast<CallingConv::ID>((0x7ff & CCInfo) >> bitc::CALL_CCONV));
       CallInst::TailCallKind TCK = CallInst::TCK_None;
       if (CCInfo & 1 << bitc::CALL_TAIL)

llvm/lib/IR/AutoUpgrade.cpp

void llvm::UpgradeAttributes(AttrBuilder &B) {
   if (A.isValid()) {
     // The value can be "true" or "false".
     bool NullPointerIsValid = A.getValueAsString() == "true";
     B.removeAttribute("null-pointer-is-valid");
     if (NullPointerIsValid)
       B.addAttribute(Attribute::NullPointerIsValid);
   }
 }
+
+void llvm::UpgradeOperandBundles(std::vector<OperandBundleDef> &Bundles) {
+
+  // clang.arc.attachedcall bundles are now required to have an operand.
+  // If they don't, it's okay to drop them entirely: when there is an operand,
+  // the "attachedcall" is meaningful and required, but without an operand,
+  // it's just a marker NOP.  Dropping it merely prevents an optimization.
+  erase_if(Bundles, [&](OperandBundleDef &OBD) {
+    return OBD.getTag() == "clang.arc.attachedcall" &&
+           OBD.inputs().empty();
+  });
+}

llvm/test/Bitcode/upgrade-arc-attachedcall-bundle.ll

+; Test that nullary clang.arc.attachedcall operand bundles are "upgraded".
+
+; RUN: llvm-dis %s.bc -o - | FileCheck %s
+; RUN: verify-uselistorder %s.bc
+
+define i8* @invalid() {
+; CHECK-LABEL: define i8* @invalid() {
+; CHECK-NEXT:   %tmp0 = call i8* @foo(){{$}}
+; CHECK-NEXT:   ret i8* %tmp0
+  %tmp0 = call i8* @foo() [ "clang.arc.attachedcall"() ]
+  ret i8* %tmp0
+}
+
+define i8* @valid() {
+; CHECK-LABEL: define i8* @valid() {
+; CHECK-NEXT:   %tmp0 = call i8* @foo() [ "clang.arc.attachedcall"(i8* (i8*)* @llvm.objc.retainAutoreleasedReturnValue) ]
+; CHECK-NEXT:   ret i8* %tmp0
+  %tmp0 = call i8* @foo() [ "clang.arc.attachedcall"(i8* (i8*)* @llvm.objc.retainAutoreleasedReturnValue) ]
+  ret i8* %tmp0
+}
+
+declare i8* @foo()
+declare i8* @llvm.objc.retainAutoreleasedReturnValue(i8*)