[CIR][NFC] Be a bit more strict on lowering logic and random cleanups

More tests for these comming soon

Author: bcardosolopes

clang/lib/CIR/CodeGen/CIRGenBuilder.cpp

@@ -42,11 +42,10 @@ mlir::Value CIRGenBuilderTy::promoteArrayIndex(const clang::TargetInfo &ti,
 
   // If this an integer, ensure that it is at least as width as the array index
   // type.
-  if (auto intTy = mlir::dyn_cast<cir::IntType>(index.getType())) {
+  if (auto intTy = mlir::dyn_cast<cir::IntType>(index.getType()))
     if (intTy.getWidth() < arrayIndexWidth)
       return cir::CastOp::create(*this, loc, arrayIndexType,
                                  cir::CastKind::integral, index);
-  }
 
   return index;
 }
@@ -57,17 +56,17 @@ mlir::Value CIRGenBuilderTy::getArrayElement(const clang::TargetInfo &ti,
                                              mlir::Value arrayPtr,
                                              mlir::Type eltTy, mlir::Value idx,
                                              bool shouldDecay) {
-  auto arrayPtrTy = mlir::dyn_cast<cir::PointerType>(arrayPtr.getType());
-  assert(arrayPtrTy && "expected pointer type");
-
   // If the array pointer is not decayed, emit a GetElementOp.
-  auto arrayTy = mlir::dyn_cast<cir::ArrayType>(arrayPtrTy.getPointee());
-  if (shouldDecay && arrayTy && arrayTy == eltTy) {
-    auto eltPtrTy =
-        getPointerTo(arrayTy.getElementType(), arrayPtrTy.getAddrSpace());
-    return cir::GetElementOp::create(*this, arrayLocEnd, eltPtrTy, arrayPtr,
-                                     promoteArrayIndex(ti, arrayLocBegin, idx));
-  }
+  if (shouldDecay)
+    if (auto arrayPtrTy = dyn_cast<cir::PointerType>(arrayPtr.getType()))
+      if (auto arrayTy = dyn_cast<cir::ArrayType>(arrayPtrTy.getPointee()))
+        if (arrayTy == eltTy) {
+          auto eltPtrTy =
+              getPointerTo(arrayTy.getElementType(), arrayPtrTy.getAddrSpace());
+          return cir::GetElementOp::create(
+              *this, arrayLocEnd, eltPtrTy, arrayPtr,
+              promoteArrayIndex(ti, arrayLocBegin, idx));
+        }
 
   // If we don't have sufficient type information, emit a PtrStrideOp.
   mlir::Value basePtr = arrayPtr;

clang/lib/CIR/Dialect/IR/CIRDialect.cpp

@@ -4119,7 +4119,7 @@ LogicalResult cir::GetMethodOp::verify() {
 //===----------------------------------------------------------------------===//
 
 LogicalResult cir::GetElementOp::verify() {
-  auto arrayTy = mlir::cast<cir::ArrayType>(getBaseType().getPointee());
+  auto arrayTy = cast<ArrayType>(getBaseType().getPointee());
   if (getElementType() != arrayTy.getElementType())
     return emitError() << "element type mismatch";
 

clang/lib/CIR/Dialect/Transforms/LifetimeCheck.cpp

@@ -1253,7 +1253,7 @@ void LifetimeCheckPass::updatePointsTo(mlir::Value addr, mlir::Value data,
     return;
   }
 
-  if (auto getElemOp = mlir::dyn_cast<cir::GetElementOp>(dataSrcOp)) {
+  if (auto getElemOp = dyn_cast<GetElementOp>(dataSrcOp)) {
     getPmap()[addr].clear();
     getPmap()[addr].insert(State::getLocalValue(getElemOp.getBase()));
     return;
@@ -1960,7 +1960,7 @@ void LifetimeCheckPass::dumpPmap(PMapType &pmap) {
   int entry = 0;
   for (auto &mapEntry : pmap) {
     llvm::errs() << "  " << entry << ": " << getVarNameFromValue(mapEntry.first)
-                 << "  => ";
+                 << "  " << "=> ";
     printPset(mapEntry.second);
     llvm::errs() << "\n";
     entry++;

clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp

@@ -1132,15 +1132,40 @@ mlir::LogicalResult CIRToLLVMGetElementOpLowering::matchAndRewrite(
         mlir::isa<mlir::LLVM::LLVMFunctionType>(elementTy))
       elementTy = mlir::IntegerType::get(ctx, 8, mlir::IntegerType::Signless);
 
-    // Zero-extend, sign-extend or trunc the index value.
+    // Zero-extend, sign-extend or trunc the pointer value.
     auto index = adaptor.getIndex();
+    auto width = mlir::cast<mlir::IntegerType>(index.getType()).getWidth();
     mlir::DataLayout LLVMLayout(op->getParentOfType<mlir::ModuleOp>());
-    if (auto layoutWidth =
-            LLVMLayout.getTypeIndexBitwidth(adaptor.getBase().getType())) {
-      bool isUnsigned = false;
-      if (auto strideTy = dyn_cast<cir::IntType>(op.getOperand(1).getType()))
-        isUnsigned = strideTy.isUnsigned();
-      index = promoteIndex(rewriter, index, *layoutWidth, isUnsigned);
+    auto layoutWidth =
+        LLVMLayout.getTypeIndexBitwidth(adaptor.getBase().getType());
+    auto indexOp = index.getDefiningOp();
+    if (indexOp && layoutWidth && width != *layoutWidth) {
+      // If the index comes from a subtraction, make sure the extension happens
+      // before it. To achieve that, look at unary minus, which already got
+      // lowered to "sub 0, x".
+      auto sub = dyn_cast<mlir::LLVM::SubOp>(indexOp);
+      auto unary =
+          dyn_cast_if_present<cir::UnaryOp>(op.getIndex().getDefiningOp());
+      bool rewriteSub =
+          unary && unary.getKind() == cir::UnaryOpKind::Minus && sub;
+      if (rewriteSub)
+        index = indexOp->getOperand(1);
+
+      // Handle the cast
+      auto llvmDstType = mlir::IntegerType::get(ctx, *layoutWidth);
+      index = getLLVMIntCast(rewriter, index, llvmDstType,
+                             op.getIndex().getType().isUnsigned(), width,
+                             *layoutWidth);
+
+      // Rewrite the sub in front of extensions/trunc
+      if (rewriteSub) {
+        index = mlir::LLVM::SubOp::create(
+            rewriter, index.getLoc(),
+            mlir::LLVM::ConstantOp::create(rewriter, index.getLoc(),
+                                           index.getType(), 0),
+            index);
+        rewriter.eraseOp(sub);
+      }
     }
 
     // Since the base address is a pointer to an aggregate, the first
@@ -3815,6 +3840,8 @@ CIRToLLVMAtomicFetchLowering::getLLVMBinop(cir::AtomicFetchKind k,
   case cir::AtomicFetchKind::Max:
   case cir::AtomicFetchKind::Min:
     llvm_unreachable("handled in buildMinMaxPostOp");
+  case cir::AtomicFetchKind::Xchg:
+    llvm_unreachable("xchg has no binop");
   }
   llvm_unreachable("Unknown atomic fetch opcode");
 }