Revert "[flang] Use proper attributes for runtime calls with 'i1' arguments/returns."

vzakhari · vzakhari · commit 1dd5ca24529b · 2022-11-18T20:12:33.000-08:00
Buildbot might be failing because of this: https://lab.llvm.org/buildbot/#/builders/65/builds/7283 This reverts commit 396ead9.
diff --git a/flang/include/flang/Optimizer/Builder/Runtime/RTBuilder.h b/flang/include/flang/Optimizer/Builder/Runtime/RTBuilder.h
@@ -20,7 +20,6 @@
 #include "flang/Common/Fortran.h"
 #include "flang/Common/uint128.h"
 #include "flang/Optimizer/Builder/FIRBuilder.h"
-#include "flang/Optimizer/Dialect/FIRDialect.h"
 #include "flang/Optimizer/Dialect/FIRType.h"
 #include "mlir/IR/BuiltinTypes.h"
 #include "mlir/IR/MLIRContext.h"
@@ -412,7 +411,7 @@ static mlir::func::FuncOp getRuntimeFunc(mlir::Location loc,
     return func;
   auto funTy = RuntimeEntry::getTypeModel()(builder.getContext());
   func = builder.createFunction(loc, name, funTy);
-  func->setAttr(FIROpsDialect::getFirRuntimeAttrName(), builder.getUnitAttr());
+  func->setAttr("fir.runtime", builder.getUnitAttr());
   return func;
 }
 
diff --git a/flang/include/flang/Optimizer/Dialect/FIRDialect.h b/flang/include/flang/Optimizer/Dialect/FIRDialect.h
@@ -37,11 +37,6 @@ class FIROpsDialect final : public mlir::Dialect {
   void printAttribute(mlir::Attribute attr,
                       mlir::DialectAsmPrinter &p) const override;
 
-  /// Return string name of fir.runtime attribute.
-  static constexpr llvm::StringRef getFirRuntimeAttrName() {
-    return "fir.runtime";
-  }
-
 private:
   // Register the Attributes of this dialect.
   void registerAttributes();
diff --git a/flang/lib/Lower/IO.cpp b/flang/lib/Lower/IO.cpp
@@ -27,7 +27,6 @@
 #include "flang/Optimizer/Builder/FIRBuilder.h"
 #include "flang/Optimizer/Builder/Runtime/RTBuilder.h"
 #include "flang/Optimizer/Builder/Todo.h"
-#include "flang/Optimizer/Dialect/FIRDialect.h"
 #include "flang/Optimizer/Support/FIRContext.h"
 #include "flang/Parser/parse-tree.h"
 #include "flang/Runtime/io-api.h"
@@ -168,8 +167,7 @@ static mlir::func::FuncOp getIORuntimeFunc(mlir::Location loc,
     return func;
   auto funTy = getTypeModel<E>()(builder.getContext());
   func = builder.createFunction(loc, name, funTy);
-  func->setAttr(fir::FIROpsDialect::getFirRuntimeAttrName(),
-                builder.getUnitAttr());
+  func->setAttr("fir.runtime", builder.getUnitAttr());
   func->setAttr("fir.io", builder.getUnitAttr());
   return func;
 }
diff --git a/flang/lib/Lower/IntrinsicCall.cpp b/flang/lib/Lower/IntrinsicCall.cpp
@@ -32,7 +32,6 @@
 #include "flang/Optimizer/Builder/Runtime/Stop.h"
 #include "flang/Optimizer/Builder/Runtime/Transformational.h"
 #include "flang/Optimizer/Builder/Todo.h"
-#include "flang/Optimizer/Dialect/FIRDialect.h"
 #include "flang/Optimizer/Dialect/FIROpsSupport.h"
 #include "flang/Optimizer/Support/FatalError.h"
 #include "flang/Runtime/entry-names.h"
@@ -1685,8 +1684,7 @@ static mlir::func::FuncOp getFuncOp(mlir::Location loc,
                                     const RuntimeFunction &runtime) {
   mlir::func::FuncOp function = builder.addNamedFunction(
       loc, runtime.symbol, runtime.typeGenerator(builder.getContext()));
-  function->setAttr(fir::FIROpsDialect::getFirRuntimeAttrName(),
-                    builder.getUnitAttr());
+  function->setAttr("fir.runtime", builder.getUnitAttr());
   return function;
 }
 
diff --git a/flang/lib/Optimizer/CodeGen/Target.cpp b/flang/lib/Optimizer/CodeGen/Target.cpp
@@ -22,19 +22,6 @@
 
 using namespace fir;
 
-namespace fir::details {
-llvm::StringRef Attributes::getIntExtensionAttrName() const {
-  // The attribute names are available via LLVM dialect interfaces
-  // like getZExtAttrName(), getByValAttrName(), etc., so we'd better
-  // use them than literals.
-  if (isZeroExt())
-    return "llvm.zeroext";
-  else if (isSignExt())
-    return "llvm.signext";
-  return {};
-}
-} // namespace fir::details
-
 // Reduce a REAL/float type to the floating point semantics.
 static const llvm::fltSemantics &floatToSemantics(const KindMapping &kindMap,
                                                   mlir::Type type) {
@@ -80,46 +67,6 @@ struct GenericTarget : public CodeGenSpecifics {
                                    /*sret=*/sret, /*append=*/!sret});
     return marshal;
   }
-
-  CodeGenSpecifics::Marshalling
-  integerArgumentType(mlir::Location loc,
-                      mlir::IntegerType argTy) const override {
-    CodeGenSpecifics::Marshalling marshal;
-    AT::IntegerExtension intExt = AT::IntegerExtension::None;
-    if (argTy.getWidth() < getCIntTypeWidth()) {
-      // isSigned() and isUnsigned() branches below are dead code currently.
-      // If needed, we can generate calls with signed/unsigned argument types
-      // to more precisely match C side (e.g. for Fortran runtime functions
-      // with 'unsigned short' arguments).
-      if (argTy.isSigned())
-        intExt = AT::IntegerExtension::Sign;
-      else if (argTy.isUnsigned())
-        intExt = AT::IntegerExtension::Zero;
-      else if (argTy.isSignless()) {
-        // Zero extend for 'i1' and sign extend for other types.
-        if (argTy.getWidth() == 1)
-          intExt = AT::IntegerExtension::Zero;
-        else
-          intExt = AT::IntegerExtension::Sign;
-      }
-    }
-
-    marshal.emplace_back(argTy, AT{/*alignment=*/0, /*byval=*/false,
-                                   /*sret=*/false, /*append=*/false,
-                                   /*intExt=*/intExt});
-    return marshal;
-  }
-
-  CodeGenSpecifics::Marshalling
-  integerReturnType(mlir::Location loc,
-                    mlir::IntegerType argTy) const override {
-    return integerArgumentType(loc, argTy);
-  }
-
-  // Width of 'int' type is 32-bits for almost all targets, except
-  // for AVR and MSP430 (see TargetInfo initializations
-  // in clang/lib/Basic/Targets).
-  unsigned char getCIntTypeWidth() const override { return 32; }
 };
 } // namespace
 
diff --git a/flang/lib/Optimizer/CodeGen/Target.h b/flang/lib/Optimizer/CodeGen/Target.h
@@ -29,29 +29,21 @@ namespace details {
 /// LLVMContext.
 class Attributes {
 public:
-  enum class IntegerExtension { None, Zero, Sign };
-
   Attributes(unsigned short alignment = 0, bool byval = false,
-             bool sret = false, bool append = false,
-             IntegerExtension intExt = IntegerExtension::None)
-      : alignment{alignment}, byval{byval}, sret{sret}, append{append},
-        intExt{intExt} {}
+             bool sret = false, bool append = false)
+      : alignment{alignment}, byval{byval}, sret{sret}, append{append} {}
 
   unsigned getAlignment() const { return alignment; }
   bool hasAlignment() const { return alignment != 0; }
   bool isByVal() const { return byval; }
   bool isSRet() const { return sret; }
   bool isAppend() const { return append; }
-  bool isZeroExt() const { return intExt == IntegerExtension::Zero; }
-  bool isSignExt() const { return intExt == IntegerExtension::Sign; }
-  llvm::StringRef getIntExtensionAttrName() const;
 
 private:
   unsigned short alignment{};
   bool byval : 1;
   bool sret : 1;
   bool append : 1;
-  IntegerExtension intExt;
 };
 
 } // namespace details
@@ -102,47 +94,6 @@ class CodeGenSpecifics {
   virtual Marshalling boxcharArgumentType(mlir::Type eleTy,
                                           bool sret = false) const = 0;
 
-  // Compute ABI rules for an integer argument of the given mlir::IntegerType
-  // \p argTy. Note that this methods is supposed to be called for
-  // arguments passed by value not via reference, e.g. the 'i1' argument here:
-  //   declare i1 @_FortranAioOutputLogical(ptr, i1)
-  //
-  // \p loc is the location of the operation using/specifying the argument.
-  //
-  // Currently, the only supported marshalling is whether the argument
-  // should be zero or sign extended.
-  //
-  // The zero/sign extension is especially important to comply with the ABI
-  // used by C/C++ compiler that builds Fortran runtime. As in the above
-  // example the callee will expect the caller to zero extend the second
-  // argument up to the size of the C/C++'s 'int' type.
-  // The corresponding handling in clang is done in
-  // DefaultABIInfo::classifyArgumentType(), and the logic may brielfy
-  // be explained as some sort of extension is required if the integer
-  // type is shorter than the size of 'int' for the target.
-  // The related code is located in ASTContext::isPromotableIntegerType()
-  // and ABIInfo::isPromotableIntegerTypeForABI().
-  // In particular, the latter returns 'true' for 'bool', several kinds
-  // of 'char', 'short', 'wchar' and enumerated types.
-  // The type of the extensions (zero or sign) depends on the signedness
-  // of the original language type.
-  //
-  // It is not clear how to handle signless integer types.
-  // From the point of Fortran-C interface all supported integer types
-  // seem to be signed except for CFI_type_Bool/bool that is supported
-  // via signless 'i1', but that is treated as unsigned type by clang
-  // (e.g. 'bool' arguments are using 'zeroext' ABI).
-  virtual Marshalling integerArgumentType(mlir::Location loc,
-                                          mlir::IntegerType argTy) const = 0;
-
-  // By default, integer argument and return values use the same
-  // zero/sign extension rules.
-  virtual Marshalling integerReturnType(mlir::Location loc,
-                                        mlir::IntegerType argTy) const = 0;
-
-  // Returns width in bits of C/C++ 'int' type size.
-  virtual unsigned char getCIntTypeWidth() const = 0;
-
 protected:
   mlir::MLIRContext &context;
   llvm::Triple triple;
diff --git a/flang/lib/Optimizer/CodeGen/TargetRewrite.cpp b/flang/lib/Optimizer/CodeGen/TargetRewrite.cpp
@@ -100,14 +100,14 @@ class TargetRewrite : public fir::impl::TargetRewritePassBase<TargetRewrite> {
     // Convert ops in target-specific patterns.
     mod.walk([&](mlir::Operation *op) {
       if (auto call = mlir::dyn_cast<fir::CallOp>(op)) {
-        if (!hasPortableSignature(call.getFunctionType(), op))
+        if (!hasPortableSignature(call.getFunctionType()))
           convertCallOp(call);
       } else if (auto dispatch = mlir::dyn_cast<fir::DispatchOp>(op)) {
-        if (!hasPortableSignature(dispatch.getFunctionType(), op))
+        if (!hasPortableSignature(dispatch.getFunctionType()))
           convertCallOp(dispatch);
       } else if (auto addr = mlir::dyn_cast<fir::AddrOfOp>(op)) {
         if (addr.getType().isa<mlir::FunctionType>() &&
-            !hasPortableSignature(addr.getType(), op))
+            !hasPortableSignature(addr.getType()))
           convertAddrOp(addr);
       }
     });
@@ -443,23 +443,19 @@ class TargetRewrite : public fir::impl::TargetRewritePassBase<TargetRewrite> {
   /// then it is considered portable for any target, and this function will
   /// return `true`. Otherwise, the signature is not portable and `false` is
   /// returned.
-  bool hasPortableSignature(mlir::Type signature, mlir::Operation *op) {
+  bool hasPortableSignature(mlir::Type signature) {
     assert(signature.isa<mlir::FunctionType>());
     auto func = signature.dyn_cast<mlir::FunctionType>();
-    bool hasFirRuntime = op->hasAttrOfType<mlir::UnitAttr>(
-        fir::FIROpsDialect::getFirRuntimeAttrName());
     for (auto ty : func.getResults())
       if ((ty.isa<fir::BoxCharType>() && !noCharacterConversion) ||
-          (fir::isa_complex(ty) && !noComplexConversion) ||
-          (ty.isa<mlir::IntegerType>() && hasFirRuntime)) {
+          (fir::isa_complex(ty) && !noComplexConversion)) {
         LLVM_DEBUG(llvm::dbgs() << "rewrite " << signature << " for target\n");
         return false;
       }
     for (auto ty : func.getInputs())
       if (((ty.isa<fir::BoxCharType>() || fir::isCharacterProcedureTuple(ty)) &&
            !noCharacterConversion) ||
-          (fir::isa_complex(ty) && !noComplexConversion) ||
-          (ty.isa<mlir::IntegerType>() && hasFirRuntime)) {
+          (fir::isa_complex(ty) && !noComplexConversion)) {
         LLVM_DEBUG(llvm::dbgs() << "rewrite " << signature << " for target\n");
         return false;
       }
@@ -480,14 +476,13 @@ class TargetRewrite : public fir::impl::TargetRewritePassBase<TargetRewrite> {
   /// the immediately subsequent target code gen.
   void convertSignature(mlir::func::FuncOp func) {
     auto funcTy = func.getFunctionType().cast<mlir::FunctionType>();
-    if (hasPortableSignature(funcTy, func) && !hasHostAssociations(func))
+    if (hasPortableSignature(funcTy) && !hasHostAssociations(func))
       return;
     llvm::SmallVector<mlir::Type> newResTys;
     llvm::SmallVector<mlir::Type> newInTys;
     llvm::SmallVector<std::pair<unsigned, mlir::NamedAttribute>> savedAttrs;
     llvm::SmallVector<std::pair<unsigned, mlir::NamedAttribute>> extraAttrs;
     llvm::SmallVector<FixupTy> fixups;
-    llvm::SmallVector<std::pair<unsigned, mlir::NamedAttrList>, 1> resultAttrs;
 
     // Save argument attributes in case there is a shift so we can replace them
     // correctly.
@@ -514,22 +509,6 @@ class TargetRewrite : public fir::impl::TargetRewritePassBase<TargetRewrite> {
             else
               doComplexReturn(func, cmplx, newResTys, newInTys, fixups);
           })
-          .Case<mlir::IntegerType>([&](mlir::IntegerType intTy) {
-            auto m = specifics->integerArgumentType(func.getLoc(), intTy);
-            assert(m.size() == 1);
-            auto attr = std::get<fir::CodeGenSpecifics::Attributes>(m[0]);
-            auto retTy = std::get<mlir::Type>(m[0]);
-            std::size_t resId = newResTys.size();
-            llvm::StringRef extensionAttrName = attr.getIntExtensionAttrName();
-            if (!extensionAttrName.empty() &&
-                // TODO: we have to do the same for BIND(C) routines.
-                func->hasAttrOfType<mlir::UnitAttr>(
-                    fir::FIROpsDialect::getFirRuntimeAttrName()))
-              resultAttrs.emplace_back(
-                  resId, rewriter->getNamedAttr(extensionAttrName,
-                                                rewriter->getUnitAttr()));
-            newResTys.push_back(retTy);
-          })
           .Default([&](mlir::Type ty) { newResTys.push_back(ty); });
 
     // Saved potential shift in argument. Handling of result can add arguments
@@ -593,26 +572,6 @@ class TargetRewrite : public fir::impl::TargetRewritePassBase<TargetRewrite> {
               newInTys.push_back(ty);
             }
           })
-          .Case<mlir::IntegerType>([&](mlir::IntegerType intTy) {
-            auto m = specifics->integerArgumentType(func.getLoc(), intTy);
-            assert(m.size() == 1);
-            auto attr = std::get<fir::CodeGenSpecifics::Attributes>(m[0]);
-            auto argTy = std::get<mlir::Type>(m[0]);
-            auto argNo = newInTys.size();
-            llvm::StringRef extensionAttrName = attr.getIntExtensionAttrName();
-            if (!extensionAttrName.empty() &&
-                // TODO: we have to do the same for BIND(C) routines.
-                func->hasAttrOfType<mlir::UnitAttr>(
-                    fir::FIROpsDialect::getFirRuntimeAttrName())) {
-              fixups.emplace_back(FixupTy::Codes::ArgumentType, argNo,
-                                  [=](mlir::func::FuncOp func) {
-                                    func.setArgAttr(
-                                        argNo, extensionAttrName,
-                                        mlir::UnitAttr::get(func.getContext()));
-                                  });
-            }
-            newInTys.push_back(argTy);
-          })
           .Default([&](mlir::Type ty) { newInTys.push_back(ty); });
 
       if (func.getArgAttrOfType<mlir::UnitAttr>(index,
@@ -649,18 +608,14 @@ class TargetRewrite : public fir::impl::TargetRewritePassBase<TargetRewrite> {
         case FixupTy::Codes::ArgumentType: {
           // Argument is pass-by-value, but its type has likely been modified to
           // suit the target ABI convention.
-          auto oldArgTy =
-              fir::ReferenceType::get(oldArgTys[fixup.index - offset]);
-          // If type did not change, keep the original argument.
-          if (newInTys[fixup.index] == oldArgTy)
-            break;
-
           auto newArg = func.front().insertArgument(fixup.index,
                                                     newInTys[fixup.index], loc);
           rewriter->setInsertionPointToStart(&func.front());
           auto mem =
               rewriter->create<fir::AllocaOp>(loc, newInTys[fixup.index]);
           rewriter->create<fir::StoreOp>(loc, newArg, mem);
+          auto oldArgTy =
+              fir::ReferenceType::get(oldArgTys[fixup.index - offset]);
           auto cast = rewriter->create<fir::ConvertOp>(loc, oldArgTy, mem);
           mlir::Value load = rewriter->create<fir::LoadOp>(loc, cast);
           func.getArgument(fixup.index + 1).replaceAllUsesWith(load);
@@ -789,10 +744,6 @@ class TargetRewrite : public fir::impl::TargetRewritePassBase<TargetRewrite> {
       func.setArgAttr(extraAttr.first, extraAttr.second.getName(),
                       extraAttr.second.getValue());
 
-    for (auto [resId, resAttrList] : resultAttrs)
-      for (mlir::NamedAttribute resAttr : resAttrList)
-        func.setResultAttr(resId, resAttr.getName(), resAttr.getValue());
-
     // Replace attributes to the correct argument if there was an argument shift
     // to the right.
     if (argumentShift > 0) {
diff --git a/flang/test/Fir/target-rewrite-integer.fir b/flang/test/Fir/target-rewrite-integer.fir
