Merged main:79382eb97adff71dbc9d4969431dd4a0967ee7ad into amd-gfx:685560350718

Local branch amd-gfx 6855603 Manual merge remote-tracking branch llvm-org/main into amd-gfx
Remote branch main 79382eb [clang][bytecode] Remove superfluous check from fixed-point div (llvm#110478)

Author: SC llvm team

clang/lib/AST/ByteCode/Compiler.cpp

@@ -1542,19 +1542,30 @@ bool Compiler<Emitter>::VisitFixedPointBinOp(const BinaryOperator *E) {
     return true;
   };
 
+  auto MaybeCastToBool = [&](bool Result) {
+    if (!Result)
+      return false;
+    PrimType T = classifyPrim(E);
+    if (DiscardResult)
+      return this->emitPop(T, E);
+    if (T != PT_Bool)
+      return this->emitCast(PT_Bool, T, E);
+    return true;
+  };
+
   switch (E->getOpcode()) {
   case BO_EQ:
-    return this->emitEQFixedPoint(E);
+    return MaybeCastToBool(this->emitEQFixedPoint(E));
   case BO_NE:
-    return this->emitNEFixedPoint(E);
+    return MaybeCastToBool(this->emitNEFixedPoint(E));
   case BO_LT:
-    return this->emitLTFixedPoint(E);
+    return MaybeCastToBool(this->emitLTFixedPoint(E));
   case BO_LE:
-    return this->emitLEFixedPoint(E);
+    return MaybeCastToBool(this->emitLEFixedPoint(E));
   case BO_GT:
-    return this->emitGTFixedPoint(E);
+    return MaybeCastToBool(this->emitGTFixedPoint(E));
   case BO_GE:
-    return this->emitGEFixedPoint(E);
+    return MaybeCastToBool(this->emitGEFixedPoint(E));
   case BO_Add:
     return ConvertResult(this->emitAddFixedPoint(E));
   case BO_Sub:

clang/lib/AST/ByteCode/Interp.h

@@ -235,14 +235,16 @@ bool CheckDivRem(InterpState &S, CodePtr OpPC, const T &LHS, const T &RHS) {
     return false;
   }
 
-  if (LHS.isSigned() && LHS.isMin() && RHS.isNegative() && RHS.isMinusOne()) {
-    APSInt LHSInt = LHS.toAPSInt();
-    SmallString<32> Trunc;
-    (-LHSInt.extend(LHSInt.getBitWidth() + 1)).toString(Trunc, 10);
-    const SourceInfo &Loc = S.Current->getSource(OpPC);
-    const Expr *E = S.Current->getExpr(OpPC);
-    S.CCEDiag(Loc, diag::note_constexpr_overflow) << Trunc << E->getType();
-    return false;
+  if constexpr (!std::is_same_v<T, FixedPoint>) {
+    if (LHS.isSigned() && LHS.isMin() && RHS.isNegative() && RHS.isMinusOne()) {
+      APSInt LHSInt = LHS.toAPSInt();
+      SmallString<32> Trunc;
+      (-LHSInt.extend(LHSInt.getBitWidth() + 1)).toString(Trunc, 10);
+      const SourceInfo &Loc = S.Current->getSource(OpPC);
+      const Expr *E = S.Current->getExpr(OpPC);
+      S.CCEDiag(Loc, diag::note_constexpr_overflow) << Trunc << E->getType();
+      return false;
+    }
   }
   return true;
 }

clang/test/Frontend/fixed_point_comparisons.c

@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,UNPADDED
 // RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -fpadding-on-unsigned-fixed-point -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,PADDED
 
+// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -emit-llvm %s -o - -fexperimental-new-constant-interpreter | FileCheck %s --check-prefixes=CHECK,UNPADDED
+// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -fpadding-on-unsigned-fixed-point -emit-llvm %s -o - -fexperimental-new-constant-interpreter | FileCheck %s --check-prefixes=CHECK,PADDED
+
 // Fixed point against other fixed point
 _Bool b_eq_true = 2.5hk == 2.5uhk;  // CHECK-DAG: @b_eq_true  = {{.*}}global i8 1, align 1
 _Bool b_eq_false = 2.5hk == 2.4uhk; // CHECK-DAG: @b_eq_false = {{.*}}global i8 0, align 1

clang/test/Frontend/fixed_point_div_const.c

@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,SIGNED
 // RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -fpadding-on-unsigned-fixed-point -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,UNSIGNED
 
+// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -emit-llvm %s -o - -fexperimental-new-constant-interpreter | FileCheck %s --check-prefixes=CHECK,SIGNED
+// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -fpadding-on-unsigned-fixed-point -emit-llvm %s -o - -fexperimental-new-constant-interpreter | FileCheck %s --check-prefixes=CHECK,UNSIGNED
+
 // Division between different fixed point types
 short _Accum sa_const = 1.0hk / 2.0hk;
 // CHECK-DAG: @sa_const  = {{.*}}global i16 64, align 2

flang/lib/Optimizer/Transforms/DebugTypeGenerator.cpp

@@ -14,7 +14,6 @@
 
 #include "DebugTypeGenerator.h"
 #include "flang/Optimizer/CodeGen/DescriptorModel.h"
-#include "flang/Optimizer/CodeGen/TypeConverter.h"
 #include "flang/Optimizer/Support/InternalNames.h"
 #include "mlir/Pass/Pass.h"
 #include "llvm/ADT/ScopeExit.h"
@@ -48,7 +47,7 @@ DebugTypeGenerator::DebugTypeGenerator(mlir::ModuleOp m,
                                        mlir::SymbolTable *symbolTable_,
                                        const mlir::DataLayout &dl)
     : module(m), symbolTable(symbolTable_), dataLayout{&dl},
-      kindMapping(getKindMapping(m)) {
+      kindMapping(getKindMapping(m)), llvmTypeConverter(m, false, false, dl) {
   LLVM_DEBUG(llvm::dbgs() << "DITypeAttr generator\n");
 
   mlir::MLIRContext *context = module.getContext();
@@ -160,29 +159,110 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertBoxedSequenceType(
       dataLocation, /*rank=*/nullptr, allocated, associated);
 }
 
+// If the type is a pointer or array type then gets its underlying type.
+static mlir::LLVM::DITypeAttr getUnderlyingType(mlir::LLVM::DITypeAttr Ty) {
+  if (auto ptrTy =
+          mlir::dyn_cast_if_present<mlir::LLVM::DIDerivedTypeAttr>(Ty)) {
+    if (ptrTy.getTag() == llvm::dwarf::DW_TAG_pointer_type)
+      Ty = getUnderlyingType(ptrTy.getBaseType());
+  }
+  if (auto comTy =
+          mlir::dyn_cast_if_present<mlir::LLVM::DICompositeTypeAttr>(Ty)) {
+    if (comTy.getTag() == llvm::dwarf::DW_TAG_array_type)
+      Ty = getUnderlyingType(comTy.getBaseType());
+  }
+  return Ty;
+}
+
+// Currently, the handling of recursive debug type in mlir has some limitations.
+// Those limitations were discussed at the end of the thread for following PR.
+// https://github.com/llvm/llvm-project/pull/106571
+//
+// Problem could be explained with the following example code:
+//  type t2
+//   type(t1), pointer :: p1
+// end type
+// type t1
+//   type(t2), pointer :: p2
+// end type
+// In the description below, type_self means a temporary type that is generated
+// as a place holder while the members of that type are being processed.
+//
+// If we process t1 first then we will have the following structure after it has
+// been processed.
+// t1 -> t2 -> t1_self
+// This is because when we started processing t2, we did not have the complete
+// t1 but its place holder t1_self.
+// Now if some entity requires t2, we will already have that in cache and will
+// return it. But this t2 refers to t1_self and not to t1. In mlir handling,
+// only those types are allowed to have _self reference which are wrapped by
+// entity whose reference it is. So t1 -> t2 -> t1_self is ok because the
+// t1_self reference can be resolved by the outer t1. But standalone t2 is not
+// because there will be no way to resolve it. Until this is fixed in mlir, we
+// avoid caching such types. Please see DebugTranslation::translateRecursive for
+// details on how mlir handles recursive types.
+static bool canCacheThisType(mlir::LLVM::DICompositeTypeAttr comTy) {
+  for (auto el : comTy.getElements()) {
+    if (auto mem =
+            mlir::dyn_cast_if_present<mlir::LLVM::DIDerivedTypeAttr>(el)) {
+      mlir::LLVM::DITypeAttr memTy = getUnderlyingType(mem.getBaseType());
+      if (auto baseTy =
+              mlir::dyn_cast_if_present<mlir::LLVM::DICompositeTypeAttr>(
+                  memTy)) {
+        // We will not cache a type if one of its member meets the following
+        // conditions:
+        // 1. It is a structure type
+        // 2. It is a place holder type (getIsRecSelf() is true)
+        // 3. It is not a self reference. It is ok to have t1_self in t1.
+        if (baseTy.getTag() == llvm::dwarf::DW_TAG_structure_type &&
+            baseTy.getIsRecSelf() && (comTy.getRecId() != baseTy.getRecId()))
+          return false;
+      }
+    }
+  }
+  return true;
+}
+
 mlir::LLVM::DITypeAttr DebugTypeGenerator::convertRecordType(
     fir::RecordType Ty, mlir::LLVM::DIFileAttr fileAttr,
     mlir::LLVM::DIScopeAttr scope, fir::cg::XDeclareOp declOp) {
+  // Check if this type has already been converted.
+  auto iter = typeCache.find(Ty);
+  if (iter != typeCache.end())
+    return iter->second;
+
+  llvm::SmallVector<mlir::LLVM::DINodeAttr> elements;
   mlir::MLIRContext *context = module.getContext();
+  auto recId = mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
+  // Generate a place holder TypeAttr which will be used if a member
+  // references the parent type.
+  auto comAttr = mlir::LLVM::DICompositeTypeAttr::get(
+      context, recId, /*isRecSelf=*/true, llvm::dwarf::DW_TAG_structure_type,
+      mlir::StringAttr::get(context, ""), fileAttr, /*line=*/0, scope,
+      /*baseType=*/nullptr, mlir::LLVM::DIFlags::Zero, /*sizeInBits=*/0,
+      /*alignInBits=*/0, elements, /*dataLocation=*/nullptr, /*rank=*/nullptr,
+      /*allocated=*/nullptr, /*associated=*/nullptr);
+  typeCache[Ty] = comAttr;
+
   auto result = fir::NameUniquer::deconstruct(Ty.getName());
   if (result.first != fir::NameUniquer::NameKind::DERIVED_TYPE)
     return genPlaceholderType(context);
 
   fir::TypeInfoOp tiOp = symbolTable->lookup<fir::TypeInfoOp>(Ty.getName());
   unsigned line = (tiOp) ? getLineFromLoc(tiOp.getLoc()) : 1;
 
-  llvm::SmallVector<mlir::LLVM::DINodeAttr> elements;
   std::uint64_t offset = 0;
   for (auto [fieldName, fieldTy] : Ty.getTypeList()) {
-    auto result = fir::getTypeSizeAndAlignment(module.getLoc(), fieldTy,
-                                               *dataLayout, kindMapping);
-    // If we get a type whose size we can't determine, we will break the loop
-    // and generate the derived type with whatever components we have
-    // assembled thus far.
-    if (!result)
-      break;
-    auto [byteSize, byteAlign] = *result;
+    mlir::Type llvmTy;
+    if (auto boxTy = mlir::dyn_cast_or_null<fir::BaseBoxType>(fieldTy))
+      llvmTy =
+          llvmTypeConverter.convertBoxTypeAsStruct(boxTy, getBoxRank(boxTy));
+    else
+      llvmTy = llvmTypeConverter.convertType(fieldTy);
+
     // FIXME: Handle non defaults array bound in derived types
+    uint64_t byteSize = dataLayout->getTypeSize(llvmTy);
+    unsigned short byteAlign = dataLayout->getTypeABIAlignment(llvmTy);
     mlir::LLVM::DITypeAttr elemTy =
         convertType(fieldTy, fileAttr, scope, /*declOp=*/nullptr);
     offset = llvm::alignTo(offset, byteAlign);
@@ -195,12 +275,20 @@ mlir::LLVM::DITypeAttr DebugTypeGenerator::convertRecordType(
     offset += llvm::alignTo(byteSize, byteAlign);
   }
 
-  return mlir::LLVM::DICompositeTypeAttr::get(
-      context, llvm::dwarf::DW_TAG_structure_type,
+  auto finalAttr = mlir::LLVM::DICompositeTypeAttr::get(
+      context, recId, /*isRecSelf=*/false, llvm::dwarf::DW_TAG_structure_type,
       mlir::StringAttr::get(context, result.second.name), fileAttr, line, scope,
       /*baseType=*/nullptr, mlir::LLVM::DIFlags::Zero, offset * 8,
       /*alignInBits=*/0, elements, /*dataLocation=*/nullptr, /*rank=*/nullptr,
       /*allocated=*/nullptr, /*associated=*/nullptr);
+  if (canCacheThisType(finalAttr)) {
+    typeCache[Ty] = finalAttr;
+  } else {
+    auto iter = typeCache.find(Ty);
+    if (iter != typeCache.end())
+      typeCache.erase(iter);
+  }
+  return finalAttr;
 }
 
 mlir::LLVM::DITypeAttr DebugTypeGenerator::convertSequenceType(

flang/lib/Optimizer/Transforms/DebugTypeGenerator.h

@@ -14,6 +14,7 @@
 #define FORTRAN_OPTIMIZER_TRANSFORMS_DEBUGTYPEGENERATOR_H
 
 #include "flang/Optimizer/CodeGen/CGOps.h"
+#include "flang/Optimizer/CodeGen/TypeConverter.h"
 #include "flang/Optimizer/Dialect/FIRType.h"
 #include "flang/Optimizer/Dialect/Support/FIRContext.h"
 #include "flang/Optimizer/Dialect/Support/KindMapping.h"
@@ -68,10 +69,12 @@ class DebugTypeGenerator {
   mlir::SymbolTable *symbolTable;
   const mlir::DataLayout *dataLayout;
   KindMapping kindMapping;
+  fir::LLVMTypeConverter llvmTypeConverter;
   std::uint64_t dimsSize;
   std::uint64_t dimsOffset;
   std::uint64_t ptrSize;
   std::uint64_t lenOffset;
+  llvm::DenseMap<mlir::Type, mlir::LLVM::DITypeAttr> typeCache;
 };
 
 } // namespace fir

flang/test/Integration/debug-cyclic-derived-type-2.f90

@@ -0,0 +1,22 @@
+! RUN: %flang_fc1 -emit-llvm -debug-info-kind=standalone %s -o - | FileCheck  %s
+
+! mainly test that this program does not cause an assertion failure
+module m
+ type t2
+   type(t1), pointer :: p1
+ end type
+ type t1
+   type(t2), pointer :: p2
+   integer abc
+ end type
+ type(t1) :: tee1
+end module
+
+program test
+  use m
+  type(t2) :: lc2
+  print *, lc2%p1%abc
+end program test
+
+! CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "t1"{{.*}})
+! CHECK-DAG: DICompositeType(tag: DW_TAG_structure_type, name: "t2"{{.*}})

flang/test/Integration/debug-cyclic-derived-type.f90

@@ -11,5 +11,11 @@ module m
  type(t2) :: v3
 end module
 
-! CHECK-DAG: !DICompositeType(tag: DW_TAG_structure_type, name: "t1"{{.*}})
-! CHECK-DAG: !DICompositeType(tag: DW_TAG_structure_type, name: "t2"{{.*}})
+! CHECK-DAG: ![[T1:[0-9]+]] = {{.*}}!DICompositeType(tag: DW_TAG_structure_type, name: "t1"{{.*}}elements: ![[T1_ELEMS:[0-9]+]])
+! CHECK-DAG: ![[T1_ELEMS]] = !{![[T1_ELEM1:[0-9]+]]}
+! CHECK-DAG: ![[T1_ELEM1]] = !DIDerivedType(tag: DW_TAG_member, name: "p", baseType: ![[T2P:[0-9]+]]{{.*}})
+! CHECK-DAG: ![[T2P]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[T2:[0-9]+]]{{.*}})
+
+! CHECK-DAG: ![[T2]] = {{.*}}!DICompositeType(tag: DW_TAG_structure_type, name: "t2"{{.*}}elements: ![[T2_ELEMS:[0-9]+]])
+! CHECK-DAG: ![[T2_ELEMS]] = !{![[T2_ELEM1:[0-9]+]]}
+! CHECK-DAG: ![[T2_ELEM1]] = !DIDerivedType(tag: DW_TAG_member, name: "v1", baseType: ![[T1]]{{.*}})