[CIR] Add alignment support for global, store, and load ops

This adds alignment support for GlobalOp, LoadOp, and StoreOp.

Tests which failed because cir.store now prints alignment were updated
with wildcard matches, except where the alignment was relevant to the
test. Tests which check for cir.store in cases that don't have explicit
alignment were not updated.

The alignment for loads from structures and unions is incorrect both here
and in the incubator. That will be fixed in a separate patch.

Author: andykaylor

clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h

@@ -184,19 +184,9 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
                                     global.getSymName());
   }
 
-  cir::LoadOp createLoad(mlir::Location loc, mlir::Value ptr,
-                         bool isVolatile = false, uint64_t alignment = 0) {
-    mlir::IntegerAttr intAttr;
-    if (alignment)
-      intAttr = mlir::IntegerAttr::get(
-          mlir::IntegerType::get(ptr.getContext(), 64), alignment);
-
-    return create<cir::LoadOp>(loc, ptr);
-  }
-
-  cir::StoreOp createStore(mlir::Location loc, mlir::Value val,
-                           mlir::Value dst) {
-    return create<cir::StoreOp>(loc, val, dst);
+  cir::StoreOp createStore(mlir::Location loc, mlir::Value val, mlir::Value dst,
+                           mlir::IntegerAttr align = {}) {
+    return create<cir::StoreOp>(loc, val, dst, align);
   }
 
   cir::GetMemberOp createGetMember(mlir::Location loc, mlir::Type resultTy,
@@ -209,7 +199,12 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
                                clang::CharUnits alignment) {
     auto addr = createAlloca(loc, getPointerTo(type), type, {},
                              getSizeFromCharUnits(getContext(), alignment));
-    return createLoad(loc, addr);
+    mlir::IntegerAttr alignAttr;
+    uint64_t align = alignment.getQuantity();
+    if (align)
+      alignAttr = getI64IntegerAttr(align);
+
+    return create<cir::LoadOp>(loc, addr, /*isDeref=*/false, alignAttr);
   }
 
   cir::PtrStrideOp createPtrStride(mlir::Location loc, mlir::Value base,

clang/include/clang/CIR/Dialect/IR/CIROps.td

@@ -417,11 +417,15 @@ def LoadOp : CIR_Op<"load", [
   }];
 
   let arguments = (ins Arg<CIR_PointerType, "the address to load from",
-                           [MemRead]>:$addr, UnitAttr:$isDeref);
+                           [MemRead]>:$addr,
+                       UnitAttr:$isDeref,
+                       OptionalAttr<I64Attr>:$alignment
+                       );
   let results = (outs CIR_AnyType:$result);
 
   let assemblyFormat = [{
     (`deref` $isDeref^)?
+    (`align` `(` $alignment^ `)`)?
     $addr `:` qualified(type($addr)) `,` type($result) attr-dict
   }];
 
@@ -458,9 +462,11 @@ def StoreOp : CIR_Op<"store", [
 
   let arguments = (ins CIR_AnyType:$value,
                        Arg<CIR_PointerType, "the address to store the value",
-                           [MemWrite]>:$addr);
+                           [MemWrite]>:$addr,
+                           OptionalAttr<I64Attr>:$alignment);
 
   let assemblyFormat = [{
+    (`align` `(` $alignment^ `)`)?
     $value `,` $addr attr-dict `:` type($value) `,` qualified(type($addr))
   }];
 
@@ -1643,7 +1649,8 @@ def GlobalOp : CIR_Op<"global"> {
                        TypeAttr:$sym_type,
                        Arg<GlobalLinkageKind, "linkage type">:$linkage,
                        OptionalAttr<AnyAttr>:$initial_value,
-                       UnitAttr:$dsolocal);
+                       UnitAttr:$dsolocal,
+                       OptionalAttr<I64Attr>:$alignment);
 
   let assemblyFormat = [{
     $linkage

clang/include/clang/CIR/MissingFeatures.h

@@ -34,11 +34,14 @@ struct MissingFeatures {
   static bool opGlobalDSOLocal() { return false; }
   static bool opGlobalThreadLocal() { return false; }
   static bool opGlobalConstant() { return false; }
-  static bool opGlobalAlignment() { return false; }
   static bool opGlobalWeakRef() { return false; }
   static bool opGlobalLinkage() { return false; }
-  static bool opGlobalSetVisitibility() { return false; }
   static bool opGlobalUnnamedAddr() { return false; }
+  static bool opGlobalSection() { return false; }
+  static bool opGlobalVisibility() { return false; }
+  static bool opGlobalDLLImportExport() { return false; }
+  static bool opGlobalPartition() { return false; }
+  static bool opGlobalCIRGlobalValueInterface() { return false; }
 
   static bool supportIFuncAttr() { return false; }
   static bool supportVisibility() { return false; }
@@ -51,7 +54,6 @@ struct MissingFeatures {
   static bool opLoadStoreTbaa() { return false; }
   static bool opLoadStoreMemOrder() { return false; }
   static bool opLoadStoreVolatile() { return false; }
-  static bool opLoadStoreAlignment() { return false; }
   static bool opLoadStoreAtomic() { return false; }
   static bool opLoadStoreObjC() { return false; }
 

clang/lib/CIR/CodeGen/CIRGenBuilder.h

@@ -9,6 +9,7 @@
 #ifndef LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENBUILDER_H
 #define LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENBUILDER_H
 
+#include "Address.h"
 #include "CIRGenTypeCache.h"
 #include "clang/CIR/MissingFeatures.h"
 
@@ -279,6 +280,26 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
     return create<cir::BinOp>(loc, cir::BinOpKind::Div, lhs, rhs);
   }
 
+  cir::LoadOp createLoad(mlir::Location loc, Address addr,
+                         bool isVolatile = false) {
+    mlir::IntegerAttr align;
+    uint64_t alignment = addr.getAlignment().getQuantity();
+    if (alignment)
+      align = getI64IntegerAttr(alignment);
+    return create<cir::LoadOp>(loc, addr.getPointer(), /*isDeref=*/false, align);
+  }
+
+  cir::StoreOp createStore(mlir::Location loc, mlir::Value val, Address dst,
+                           ::mlir::IntegerAttr align = {}) {
+    if (!align) {
+      uint64_t alignment = dst.getAlignment().getQuantity();
+      if (alignment)
+        align = mlir::IntegerAttr::get(mlir::IntegerType::get(getContext(), 64),
+                                       alignment);
+    }
+    return CIRBaseBuilderTy::createStore(loc, val, dst.getPointer(), align);
+  }
+
   /// Create a cir.ptr_stride operation to get access to an array element.
   /// \p idx is the index of the element to access, \p shouldDecay is true if
   /// the result should decay to a pointer to the element type.

clang/lib/CIR/CodeGen/CIRGenExpr.cpp

@@ -209,10 +209,10 @@ void CIRGenFunction::emitStoreThroughLValue(RValue src, LValue dst,
       // Read/modify/write the vector, inserting the new element
       const mlir::Location loc = dst.getVectorPointer().getLoc();
       const mlir::Value vector =
-          builder.createLoad(loc, dst.getVectorAddress().getPointer());
+          builder.createLoad(loc, dst.getVectorAddress());
       const mlir::Value newVector = builder.create<cir::VecInsertOp>(
           loc, vector, src.getScalarVal(), dst.getVectorIdx());
-      builder.createStore(loc, newVector, dst.getVectorAddress().getPointer());
+      builder.createStore(loc, newVector, dst.getVectorAddress());
       return;
     }
 
@@ -301,7 +301,7 @@ void CIRGenFunction::emitStoreOfScalar(mlir::Value value, Address addr,
   }
 
   assert(currSrcLoc && "must pass in source location");
-  builder.createStore(*currSrcLoc, value, addr.getPointer() /*, isVolatile*/);
+  builder.createStore(*currSrcLoc, value, addr /*, isVolatile*/);
 
   if (isNontemporal) {
     cgm.errorNYI(addr.getPointer().getLoc(), "emitStoreOfScalar nontemporal");
@@ -409,12 +409,10 @@ mlir::Value CIRGenFunction::emitLoadOfScalar(LValue lvalue,
   Address addr = lvalue.getAddress();
   mlir::Type eltTy = addr.getElementType();
 
-  mlir::Value ptr = addr.getPointer();
   if (mlir::isa<cir::VoidType>(eltTy))
     cgm.errorNYI(loc, "emitLoadOfScalar: void type");
 
-  mlir::Value loadOp = builder.CIRBaseBuilderTy::createLoad(
-      getLoc(loc), ptr, false /*isVolatile*/);
+  mlir::Value loadOp = builder.createLoad(getLoc(loc), addr);
 
   return loadOp;
 }
@@ -431,7 +429,7 @@ RValue CIRGenFunction::emitLoadOfLValue(LValue lv, SourceLocation loc) {
 
   if (lv.isVectorElt()) {
     const mlir::Value load =
-        builder.createLoad(getLoc(loc), lv.getVectorAddress().getPointer());
+        builder.createLoad(getLoc(loc), lv.getVectorAddress());
     return RValue::get(builder.create<cir::VecExtractOp>(getLoc(loc), load,
                                                          lv.getVectorIdx()));
   }
@@ -745,11 +743,12 @@ CIRGenFunction::emitArraySubscriptExpr(const clang::ArraySubscriptExpr *e) {
 
 LValue CIRGenFunction::emitStringLiteralLValue(const StringLiteral *e) {
   cir::GlobalOp globalOp = cgm.getGlobalForStringLiteral(e);
-  assert(!cir::MissingFeatures::opGlobalAlignment());
+  assert(globalOp.getAlignment() && "expected alignment for string literal");
+  unsigned align = *(globalOp.getAlignment());
   mlir::Value addr =
       builder.createGetGlobal(getLoc(e->getSourceRange()), globalOp);
   return makeAddrLValue(
-      Address(addr, globalOp.getSymType(), CharUnits::fromQuantity(1)),
+      Address(addr, globalOp.getSymType(), CharUnits::fromQuantity(align)),
       e->getType(), AlignmentSource::Decl);
 }
 

clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp

@@ -163,7 +163,7 @@ void AggExprEmitter::emitArrayInit(Address destPtr, cir::ArrayType arrayTy,
     // TODO(CIR): Replace this part later with cir::DoWhileOp
     for (unsigned i = numInitElements; i != numArrayElements; ++i) {
       cir::LoadOp currentElement =
-          builder.createLoad(loc, tmpAddr.getPointer());
+          builder.createLoad(loc, tmpAddr);
 
       // Emit the actual filler expression.
       const LValue elementLV = cgf.makeAddrLValue(

clang/lib/CIR/CodeGen/CIRGenFunction.cpp

@@ -607,7 +607,7 @@ void CIRGenFunction::emitNullInitialization(mlir::Location loc, Address destPtr,
   // respective address.
   // Builder.CreateMemSet(DestPtr, Builder.getInt8(0), SizeVal, false);
   const mlir::Value zeroValue = builder.getNullValue(convertType(ty), loc);
-  builder.createStore(loc, zeroValue, destPtr.getPointer());
+  builder.createStore(loc, zeroValue, destPtr);
 }
 
 } // namespace clang::CIRGen

clang/lib/CIR/CodeGen/CIRGenModule.cpp

@@ -366,6 +366,39 @@ CIRGenModule::getOrCreateCIRGlobal(StringRef mangledName, mlir::Type ty,
       CIRGenModule::createGlobalOp(*this, loc, mangledName, ty,
                                    /*insertPoint=*/entry.getOperation());
 
+  // Handle things which are present even on external declarations.
+  if (d) {
+    if (langOpts.OpenMP && !langOpts.OpenMPSimd)
+      errorNYI(d->getSourceRange(), "OpenMP target global variable");
+
+    gv.setAlignmentAttr(getSize(astContext.getDeclAlign(d)));
+    assert(!cir::MissingFeatures::opGlobalConstant());
+    assert(!cir::MissingFeatures::opGlobalLinkage());
+
+    if (d->getTLSKind())
+      errorNYI(d->getSourceRange(), "thread local global variable");
+
+    assert(!cir::MissingFeatures::opGlobalDLLImportExport());
+    assert(!cir::MissingFeatures::opGlobalPartition());
+    assert(!cir::MissingFeatures::setDSOLocal());
+
+    // If required by the ABI, treat declarations of static data members with
+    // inline initializers as definitions.
+    if (astContext.isMSStaticDataMemberInlineDefinition(d))
+      errorNYI(d->getSourceRange(), "MS static data member inline definition");
+
+    assert(!cir::MissingFeatures::opGlobalSection());
+    assert(!cir::MissingFeatures::opGlobalVisibility());
+
+    // Handle XCore specific ABI requirements.
+    if (getTriple().getArch() == llvm::Triple::xcore)
+      errorNYI(d->getSourceRange(), "XCore specific ABI requirements");
+
+    // We need to check for external const declarations with initializers here,
+    // but the 'isPublic()' part of the check uses the CIRGlobalValueInterface.
+    assert(!cir::MissingFeatures::opGlobalCIRGlobalValueInterface());
+  }
+
   return gv;
 }
 
@@ -775,7 +808,8 @@ CIRGenModule::getCIRLinkageVarDefinition(const VarDecl *vd, bool isConstant) {
 
 static cir::GlobalOp generateStringLiteral(mlir::Location loc,
                                            mlir::TypedAttr c, CIRGenModule &cgm,
-                                           StringRef globalName) {
+                                           StringRef globalName,
+                                           CharUnits alignment) {
   assert(!cir::MissingFeatures::addressSpace());
 
   // Create a global variable for this string
@@ -784,7 +818,7 @@ static cir::GlobalOp generateStringLiteral(mlir::Location loc,
       CIRGenModule::createGlobalOp(cgm, loc, globalName, c.getType());
 
   // Set up extra information and add to the module
-  assert(!cir::MissingFeatures::opGlobalAlignment());
+  gv.setAlignmentAttr(cgm.getSize(alignment));
   assert(!cir::MissingFeatures::opGlobalLinkage());
   assert(!cir::MissingFeatures::opGlobalThreadLocal());
   assert(!cir::MissingFeatures::opGlobalUnnamedAddr());
@@ -821,6 +855,9 @@ std::string CIRGenModule::getUniqueGlobalName(const std::string &baseName) {
 /// Return a pointer to a constant array for the given string literal.
 cir::GlobalOp CIRGenModule::getGlobalForStringLiteral(const StringLiteral *s,
                                                       StringRef name) {
+  CharUnits alignment =
+      astContext.getAlignOfGlobalVarInChars(s->getType(), /*VD=*/nullptr);
+
   mlir::Attribute c = getConstantArrayFromStringLiteral(s);
 
   if (getLangOpts().WritableStrings) {
@@ -842,8 +879,8 @@ cir::GlobalOp CIRGenModule::getGlobalForStringLiteral(const StringLiteral *s,
   std::string uniqueName = getUniqueGlobalName(name.str());
   mlir::Location loc = getLoc(s->getSourceRange());
   auto typedC = llvm::cast<mlir::TypedAttr>(c);
-  assert(!cir::MissingFeatures::opGlobalAlignment());
-  cir::GlobalOp gv = generateStringLiteral(loc, typedC, *this, uniqueName);
+  cir::GlobalOp gv =
+      generateStringLiteral(loc, typedC, *this, uniqueName, alignment);
   assert(!cir::MissingFeatures::opGlobalDSOLocal());
 
   assert(!cir::MissingFeatures::sanitizers());
@@ -918,7 +955,7 @@ void CIRGenModule::emitTopLevelDecl(Decl *decl) {
 void CIRGenModule::setInitializer(cir::GlobalOp &op, mlir::Attribute value) {
   // Recompute visibility when updating initializer.
   op.setInitialValueAttr(value);
-  assert(!cir::MissingFeatures::opGlobalSetVisitibility());
+  assert(!cir::MissingFeatures::opGlobalVisibility());
 }
 
 cir::FuncOp CIRGenModule::getAddrOfFunction(clang::GlobalDecl gd,

clang/lib/CIR/CodeGen/CIRGenStmt.cpp

@@ -391,7 +391,8 @@ mlir::LogicalResult CIRGenFunction::emitReturnStmt(const ReturnStmt &s) {
     // If this function returns a reference, take the address of the
     // expression rather than the value.
     RValue result = emitReferenceBindingToExpr(rv);
-    builder.createStore(loc, result.getScalarVal(), *fnRetAlloca);
+    builder.CIRBaseBuilderTy::createStore(loc, result.getScalarVal(),
+                                          *fnRetAlloca);
   } else {
     mlir::Value value = nullptr;
     switch (CIRGenFunction::getEvaluationKind(rv->getType())) {

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

@@ -353,6 +353,12 @@ uint64_t RecordType::getElementOffset(const ::mlir::DataLayout &dataLayout,
     offset += dataLayout.getTypeSize(ty);
   }
 
+  // Account for padding, if necessary, for the alignment of the field whose
+  // offset we are calculating.
+  const llvm::Align tyAlign = llvm::Align(
+      getPacked() ? 1 : dataLayout.getTypeABIAlignment(members[idx]));
+  offset = llvm::alignTo(offset, tyAlign);
+
   return offset;
 }